Vonmisesfix #4649
Vonmisesfix #4649
Conversation
|
Thanks, merged! |
ev-br
added
scipy.stats
defect
| @@ -30,7 +30,7 @@ cdef double von_mises_cdf_series(double k,double x,unsigned int p): | |||
| return 0.5 + x / (2 * NPY_PI) + V / NPY_PI | |||
|
|
|||
| cdef von_mises_cdf_normalapprox(k, x): | |||
| b = sqrt(NPY_2_PI) * np.exp(k) / i0(k) | |||
| b = sqrt(NPY_2_PI) / i0e(k) # Check for negative k | |||
There was a problem hiding this comment.
I think the idea is that the new code is not equivalent to the old code when k is negative, because an absolute value is taken. But for the Von-Mises distribution k must be non-negative anyway so it's OK.
There was a problem hiding this comment.
I guess this is checked somewhere else in the stats infrastructure then.
There was a problem hiding this comment.
Actually the comment can go - you only get to this branch if k>50. But in fact the von Mises distribution does make sense for k<0 (same as that for -k but shifted by pi), and this is not really implemented in scipy.
There was a problem hiding this comment.
Should it be? That seems like a relatively straightforward change.
|
docstrings would be nice to know what k and x are kappa is the dispersion shape parameter for vonmises, AFAIR, and should be > 0 by default argument checking in the distribution |
The von Mises distribution CDF doesn't work properly for narrow distributions because the exponential and the Bessel function it is divided by both underflow; scipy.special.i0e exists for this very reason. Wrote a test and fixed the problem.