| msg181588 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-02-07 09:51 |
| random.vonmisesvariate(mu, kappa) returns a value in the range (mu%2pi)-pi/2 to (mu%2pi)+pi/2 for kappa > 1e-6. For kappa <= 1e-6 it returns an uniform random value over the range 0 to 2*pi. |
|
|
| msg181590 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-02-07 10:44 |
| I'll take a look at this. |
|
|
| msg181666 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-02-08 11:37 |
| Sorry, I was wrong. I missed that z is in range -1..1. Original report is invalid, random.vonmisesvariate() always returns a value on the full circle. However there is some inconsistency. For small kappa (<= 1e-6) result range is 0 to 2pi, for other kappa it is (mu%2pi)-pi to (mu%2pi)+pi. For consistency we should either shift a range for small kappa: if kappa <= 1e-6: return (mu % TWOPI) - _pi + TWOPI * random() or normalize a result in another case: if u3 > 0.5: theta = (mu + _acos(f)) % TWOPI else: theta = (mu - _acos(f)) % TWOPI |
|
|
| msg181673 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-02-08 15:02 |
| Agreed that this seems inconsistent. The current normalization for non-small kappa is a little odd: e.g, if mu is small and negative (-0.01, say), then we get a range that goes roughly from pi to 3*pi, when a range from -pi to pi would have made more sense. Any of (1) returning values in the range [mu - pi, mu+pi], (2) returning values in the range [-pi, pi], or (3) returning values in the range [0, 2*pi] would seem reasonable. Unassigning (the original problem is solved by not being there!) |
|
|
| msg181793 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-02-10 12:19 |
| I suspect that this is simply an error in the original code: the docstring says that mu should be in the range [0, 2*pi), so reducing mu modulo 2*pi makes little sense. I guess the lines at the end of the method were intended to be written: if u3 >= 0.5: theta = (mu + _acos(f)) % TWOPI else: theta = (mu - _acos(f)) % TWOPI instead of: if u3 >= 0.5: theta = (mu % TWOPI) + _acos(f) else: theta = (mu % TWOPI) - _acos(f) That would then give consistent results, at least. |
|
|
| msg181796 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-02-10 12:33 |
| Patch. |
|
|
| msg181799 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-02-10 12:50 |
| The message for changeset r6370f1593c72 (which introduced the incorrect code) confirms the intentions. I'll apply this patch shortly. iwasawa:cpython mdickinson$ hg log -v -r6370f1593c72 changeset: 7881:6370f1593c72 branch: legacy-trunk user: Guido van Rossum <guido@python.org> date: Mon Apr 06 14:12:13 1998 +0000 files: Lib/random.py description: Correction to vonmisesvariate() by Magnus Kessler: it should take and return something between 0 and 2*pi. Also added a reference to the literature. |
|
|
| msg181802 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-02-10 13:23 |
| Updated patch (thanks Serhiy for reviewing). |
|
|
| msg181803 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-02-10 13:32 |
| LGTM. |
|
|
| msg181807 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-02-10 14:17 |
| New changeset 6a3d18cede49 by Mark Dickinson in branch '2.7': Issue #17149: Fix random.vonmisesvariate to always return results in [0, 2*math.pi]. http://hg.python.org/cpython/rev/6a3d18cede49 New changeset 41e97652a9f9 by Mark Dickinson in branch '3.2': Issue #17149: Fix random.vonmisesvariate to always return results in [0, 2*math.pi]. http://hg.python.org/cpython/rev/41e97652a9f9 New changeset e9b4f2927412 by Mark Dickinson in branch '3.3': Issue #17149: merge fix from 3.2. http://hg.python.org/cpython/rev/e9b4f2927412 New changeset 2704e11da558 by Mark Dickinson in branch 'default': Issue #17149: merge fix from 3.3. http://hg.python.org/cpython/rev/2704e11da558 |
|
|