Conversation
|
@NAThompson This should be good for review. The only major outstanding change from the last cut at this was removing all the double precision magic numbers which I have. I would have just made the edits on the linked PR, but since the guy deleted his account I can't access the branch. |
|
|
||
| [h4 Member Functions] | ||
|
|
||
| von_mises_distribution(RealType mean = 0, RealType concentration = 1); ; |
There was a problem hiding this comment.
@jzmaddock : IIRC we now use Real instead of RealType? Obviously a nitpick.
| All the [link math_toolkit.dist_ref.nmp usual non-member accessor functions] | ||
| that are generic to all distributions are supported: __usual_accessors. | ||
|
|
||
| The domain of the random variable is O <= x <= 2[pi]. |
There was a problem hiding this comment.
Maybe use “≤” (U+2264) which I think is \u2264 in .qbk?
| { | ||
| *result = policies::raise_domain_error<RealType>( | ||
| function, | ||
| "Angle parameter is %1%, but must be between -pi and +pi!", angle, pol); |
There was a problem hiding this comment.
If the CDF is periodic, is it sensible to restrict the angle like this?
|
|
||
| [h4 Accuracy] | ||
|
|
||
| The CDF of this distribution is not analytic so an approximation is made. |
There was a problem hiding this comment.
Does the approximation mean that multiprecision types should be rejected?
| // Check symmetry of PDF and CDF | ||
| test_symmetry(0.0F); | ||
| test_symmetry(0.0); | ||
| test_symmetry(0.0L); |
There was a problem hiding this comment.
Might we test the quadrature of the pdf = 1?
|
@mborland : I've approved, but with the caveat that I don't know much about this. We'll probably want a second review from @jzmaddock . |
|
Any update on this? |
3 participants
#330 looked very close to completion so this is my attempt to foster it. Also see #296