classification
Title: random.vonmisesvariate() results range is inconsistent for small and not small kappa
Type: behavior Stage: commit review
Components: Library (Lib) Versions: Python 3.4, Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: mark.dickinson, python-dev, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2013-02-07 09:51 by serhiy.storchaka, last changed 2013-02-10 14:18 by mark.dickinson. This issue is now closed.

Files
File name Uploaded Description Edit
issue17149.patch mark.dickinson, 2013-02-10 12:33 review
issue17149_v2.patch mark.dickinson, 2013-02-10 13:24 review
Messages (10)
msg181588 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) 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) * (Python committer) Date: 2013-02-07 10:44
I'll take a look at this.
msg181666 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2013-02-10 12:33
Patch.
msg181799 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) 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) * (Python committer) Date: 2013-02-10 13:23
Updated patch (thanks Serhiy for reviewing).
msg181803 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) 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
History
Date User Action Args
2013-02-10 14:18:18mark.dickinsonsetstatus: open -> closed
resolution: fixed
2013-02-10 14:17:41python-devsetnosy: + python-dev
messages: + msg181807
2013-02-10 13:32:38serhiy.storchakasetmessages: + msg181803
2013-02-10 13:24:03mark.dickinsonsetfiles: + issue17149_v2.patch
2013-02-10 13:23:54mark.dickinsonsetmessages: + msg181802
2013-02-10 12:51:00mark.dickinsonsetassignee: mark.dickinson
messages: + msg181799
2013-02-10 12:33:55mark.dickinsonsetstage: commit review
2013-02-10 12:33:44mark.dickinsonsetfiles: + issue17149.patch
keywords: + patch
messages: + msg181796
2013-02-10 12:19:23mark.dickinsonsetmessages: + msg181793
2013-02-08 15:02:24mark.dickinsonsetassignee: mark.dickinson -> (no value)
messages: + msg181673
2013-02-08 11:37:12serhiy.storchakasetmessages: + msg181666
title: random.vonmisesvariate() returns a value only on the half-circle -> random.vonmisesvariate() results range is inconsistent for small and not small kappa
2013-02-07 10:44:39mark.dickinsonsetassignee: mark.dickinson

messages: + msg181590
nosy: + mark.dickinson
2013-02-07 09:51:27serhiy.storchakasettype: behavior
2013-02-07 09:51:08serhiy.storchakacreate