classification
Title: random.triangular error when low = high=mode
Type: behavior Stage: needs patch
Components: Library (Lib) Versions: Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: Chaka_bum, asvetlov, larry, mark108, python-dev, rhettinger, serhiy.storchaka, skrah, terry.reedy
Priority: low Keywords: easy, patch

Created on 2011-11-06 10:56 by mark108, last changed 2014-05-26 08:35 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
random_triangular.patch serhiy.storchaka, 2013-02-06 14:27 review
issue_13355.patch Chaka_bum, 2013-04-07 11:58 review
fix_triangular.diff rhettinger, 2014-05-24 00:43 Catch ZeroDivisionError review
Messages (38)
msg147148 - (view) Author: Mark (mark108) Date: 2011-11-06 10:56
When low and mode are the same in random.triangular it gives the following error:


<type 'exceptions.ZeroDivisionError'>: float division
      args = ('float division',)
      message = 'float division' 


When high and mode are the same there is no problem.
msg147149 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2011-11-06 11:09
I can't reproduce this:

Python 2.7.2 |EPD 7.1-2 (32-bit)| (default, Jul  3 2011, 15:40:35) 
[GCC 4.0.1 (Apple Inc. build 5493)] on darwin
Type "packages", "demo" or "enthought" for more information.
>>> from random import triangular
>>> triangular(low=1, high=2, mode=1)  # low == mode
1.185344240827765

Note that the order of the parameters to random.triangular is (low, high, mode).  I suspect that you're actually passing identical values for `low` and `high`, and that's what's causing the error.
msg147150 - (view) Author: Mark (mark108) Date: 2011-11-06 11:27
Many thanks, Mark. I'm very new to python so apologies for my obvious mistake (you were absolutely right, I was feeding the high and mode in back to front). 

As a separate aside, it would be convenient if low=high=mode returned low (or mode or high) rather than error but it's a minor point really and easy to work around as is.

Many thanks for your help.
msg147155 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2011-11-06 12:22
> it would be convenient if low=high=mode returned low (or mode or high)

Yes, I agree that random.triangular should degrade gracefully, in the same way that random.uniform does.
msg147548 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-11-13 04:11
3.2 doc entry:

random.triangular(low, high, mode) 
Return a random floating point number N such that low <= N <= high and with the specified mode between those bounds. The low and high bounds default to zero and one. The mode argument defaults to the midpoint between the bounds, giving a symmetric distribution.

3.2 behavior:
>>> from random import triangular
>>> triangular(1,1)
1.0
>>> triangular(1,1,1)
Traceback (most recent call last):
  File "<pyshell#2>", line 1, in <module>
    triangular(1,1,1)
  File "C:\Programs\Python32\lib\random.py", line 346, in triangular
    c = 0.5 if mode is None else (mode - low) / (high - low)
ZeroDivisionError: division by zero

I regard is as a bug that explicitly giving a 'default value' causes the function to fail. 

The last sentence of the doc is a lie: the actual default for mode is None: 

>>> triangular(1,1,None)
1.0

and if it is None, it *not* calculated (low + .5(high-low)). The actual internal third parameter is the fraction of the range (high-low) that is the up slope versus the down slope of the distribution. The code calls that 'c', as calculated by the line shown in the traceback. The fix is simple: add 'or low==high' to the condition.

    c = 0.5 if (mode is None or low==high) else (mode - low) / (high - low)

Contrary to the doc ('mode between those bounds'), the definition on Wikipedia and code include the degenerate cases of mode == low or high. The code in effect treats modes outside the range as being at an endpoint.

Suggested doc revision, with defaults given in the signature as normal:

random.triangular(low=0.0, high=1.0, mode=None) 
Return a random floating point number N from a triangular distribution such that low <= N <= high with the specified mode between or at those bounds. A mode outside a bound is treated as being at the bound. The default mode argument corresponds to the midpoint between the bounds, giving a symmetric distribution.
msg147564 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2011-11-13 18:03
I've got this one.
msg181512 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-06 12:44
Here is a patch.
msg181522 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2013-02-06 13:30
> Here is a patch.

Where?  :-)
msg181525 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-06 14:27
Now it is here.
msg181527 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2013-02-06 15:00
Looks fine to me.  Raymond: can this be applied?
msg181528 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2013-02-06 15:02
One minor comment: I'd prefer it if the second test were "elif low == high:", since that more obviously guards against the division by zero.
msg181532 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-06 15:37
> One minor comment: I'd prefer it if the second test were "elif low ==
> high:", since that more obviously guards against the division by zero.

It is written deliberately. What if low == high != mode?
msg181539 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2013-02-06 16:09
> What if low == high != mode?

Then we've got garbage in, garbage out;  that case doesn't worry me.
msg181544 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-06 17:48
An exception is better than a garbage result. But I agree, triangular() 
currently is not protectet against a situation when mode is not in low--high 
range.
msg181568 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2013-02-06 20:02
> An exception is better than a garbage result.

Agreed.  And ZeroDivisionError is the wrong exception, too---ValueError would be better.  But I'm content that the current patch fixes the immediate issue.
msg181654 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2013-02-08 03:01
I'll look at the patch shortly.  At first glance, it looks over-engineered to me.
msg186197 - (view) Author: Yuriy Senko (Chaka_bum) * Date: 2013-04-07 11:58
Added validation of input data. Check whether low <= mode <= high. If low == high return low as a result.
msg186571 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2013-04-11 18:16
Test for the issue_13355.patch is incorrect, please fix.
msg186678 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-04-12 20:31
New changeset c40e36a49033 by Andrew Svetlov in branch '3.3':
Issue #13355: Raise ValueError on random.triangular call with invalid params.
http://hg.python.org/cpython/rev/c40e36a49033

New changeset 613eb432b152 by Andrew Svetlov in branch 'default':
Issue #13355: Raise ValueError on random.triangular call with invalid params.
http://hg.python.org/cpython/rev/613eb432b152
msg186680 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-04-12 20:39
New changeset 1062c66e9bdc by Andrew Svetlov in branch '2.7':
Issue #13355: Raise ValueError on random.triangular call with invalid params.
http://hg.python.org/cpython/rev/1062c66e9bdc
msg186681 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2013-04-12 20:44
Pushed. 
Yuriy Senko, thanks for patch. 
But, please, be more accurate with tests for another one.
msg186694 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2013-04-13 06:03
I haven't had a chance to look at this one yet and am reopening.

The triangular code was originally written so that low and high could be reversed and it would still work.  I don't want to break any code that might be relying on that.

Andrew Svetlov, this patch shouldn't be committed.  I'm the maintainer of the random module and the person who wrote the original code for triangular.  The bug report was assigned to me to take care of when I got a chance.  Please be less aggressive with the commits.
msg186699 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2013-04-13 08:20
Andrew Svetlov, please revert the commit.  It breaks code that may have been working before the commit.   Also, I don't want to change the exceptions being raised in old versions of Python.
msg186721 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2013-04-13 14:36
I agree that the issue_13355.patch commit should be reverted: the code used to work fine in the case high < mode < low, and now does not.  (Similarly, a call to random.uniform(2.0, 1.0) works as expected at the moment.)

Really, I think all that's needed here is Terry's suggested one-line fix.
msg186728 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-04-13 15:07
New changeset e948154af406 by Andrew Svetlov in branch '3.3':
Revert changes for #13355 by request from Raymond Hettinger
http://hg.python.org/cpython/rev/e948154af406

New changeset 39bbbf5d7b01 by Andrew Svetlov in branch 'default':
Revert changes for #13355 by request from Raymond Hettinger
http://hg.python.org/cpython/rev/39bbbf5d7b01

New changeset 620c691c12c5 by Andrew Svetlov in branch '2.7':
Revert changes for #13355 by request from Raymond Hettinger
http://hg.python.org/cpython/rev/620c691c12c5
msg186730 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2013-04-13 15:08
Reverted. Sorry.
msg187836 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2013-04-26 07:24
Raymond:  have you had time to look at this yet?
msg187839 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2013-04-26 08:15
Soonish
msg207792 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-09 20:37
Raymond, could you please make a decision or delegate this issue to Mark, Terry, Andrew or me?
msg207913 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-01-11 20:46
[Serhiy]
> Raymond, could you please make a decision

Yes, I will this week.
msg209273 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-26 03:49
I haven't looked at this in depth but it sounds like this is a legitimate concern.  I'd like it fixed for 3.4, preferably before rc1.
msg219015 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-05-24 00:43
Attaching patch
msg219019 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-05-24 02:45
So now triangular(10, 10, 20) will always return 10?
msg219033 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2014-05-24 11:35
While NumPy is of course not normative, this is what they do:

>>> numpy.random.triangular(left=1, right=2, mode=0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "mtrand.pyx", line 3218, in mtrand.RandomState.triangular (numpy/random/mtrand/mtrand.c:13407)
ValueError: left > mode
>>> numpy.random.triangular(left=1, right=2, mode=3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "mtrand.pyx", line 3220, in mtrand.RandomState.triangular (numpy/random/mtrand/mtrand.c:13433)
ValueError: mode > right
>>> numpy.random.triangular(left=1, right=1, mode=1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "mtrand.pyx", line 3222, in mtrand.RandomState.triangular (numpy/random/mtrand/mtrand.c:13459)
ValueError: left == right
msg219112 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-05-25 23:55
Thanks Stefan.   For us, I don't see the need to add a restriction, possibly breaking code that is currently working fine (with high < mode <= low).  The important part is that we now allow low==mode or high==mode and  have a smooth transition to low==high.
msg219113 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-05-26 00:28
New changeset 7ea6c8eb91e2 by Raymond Hettinger in branch '3.4':
Issue 13355:  Make random.triangular degrade gracefully when low == high.
http://hg.python.org/cpython/rev/7ea6c8eb91e2
msg219114 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-05-26 00:40
New changeset 6dc5c4ba7544 by Raymond Hettinger in branch '2.7':
Issue 13355:  Make random.triangular degrade gracefully when low == high.
http://hg.python.org/cpython/rev/6dc5c4ba7544
msg219145 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-05-26 08:35
Note the catch on 2.7. triangular(10, 10.0) returns 10.0, but triangular(10, 10.0, 10.0) returns 10. If then you divide by the result...

I proposed change "return low" to "return low + 0.0".
History
Date User Action Args
2014-05-26 08:35:02serhiy.storchakasetmessages: + msg219145
2014-05-26 00:41:31rhettingersetstatus: open -> closed
resolution: fixed
2014-05-26 00:40:33python-devsetmessages: + msg219114
2014-05-26 00:28:46python-devsetmessages: + msg219113
2014-05-25 23:55:48rhettingersetmessages: + msg219112
2014-05-24 11:35:54skrahsetnosy: + skrah
messages: + msg219033
2014-05-24 02:45:18serhiy.storchakasetmessages: + msg219019
2014-05-24 00:43:22rhettingersetfiles: + fix_triangular.diff

messages: + msg219015
2014-01-26 03:49:09larrysetnosy: + larry
messages: + msg209273
2014-01-11 20:46:15rhettingersetmessages: + msg207913
2014-01-09 20:37:47serhiy.storchakasetmessages: + msg207792
2013-05-28 15:59:44mark.dickinsonsetnosy: - mark.dickinson
2013-05-28 15:52:11serhiy.storchakasetstage: resolved -> needs patch
2013-04-26 08:15:27rhettingersetpriority: normal -> low

messages: + msg187839
2013-04-26 07:24:35mark.dickinsonsetmessages: + msg187836
2013-04-13 15:08:11asvetlovsetmessages: + msg186730
2013-04-13 15:07:27python-devsetmessages: + msg186728
2013-04-13 14:36:07mark.dickinsonsetmessages: + msg186721
2013-04-13 08:20:39rhettingersetresolution: fixed -> (no value)
messages: + msg186699
2013-04-13 06:03:46rhettingersetstatus: closed -> open

messages: + msg186694
2013-04-12 20:44:58asvetlovsetstatus: open -> closed
resolution: fixed
messages: + msg186681

stage: patch review -> resolved
2013-04-12 20:39:43python-devsetmessages: + msg186680
2013-04-12 20:31:12python-devsetnosy: + python-dev
messages: + msg186678
2013-04-11 18:16:05asvetlovsetmessages: + msg186571
2013-04-11 17:53:26asvetlovsetversions: - Python 3.2
2013-04-07 11:58:17Chaka_bumsetfiles: + issue_13355.patch
nosy: + Chaka_bum, asvetlov
messages: + msg186197

2013-02-08 03:01:26rhettingersetmessages: + msg181654
2013-02-06 20:02:15mark.dickinsonsetmessages: + msg181568
2013-02-06 17:48:52serhiy.storchakasetmessages: + msg181544
2013-02-06 16:09:33mark.dickinsonsetmessages: + msg181539
2013-02-06 15:37:42serhiy.storchakasetmessages: + msg181532
2013-02-06 15:02:54mark.dickinsonsetmessages: + msg181528
2013-02-06 15:00:49mark.dickinsonsetmessages: + msg181527
2013-02-06 14:27:23serhiy.storchakasetfiles: + random_triangular.patch

messages: + msg181525
2013-02-06 13:30:12mark.dickinsonsetmessages: + msg181522
2013-02-06 12:45:07serhiy.storchakasetstage: needs patch -> patch review
2013-02-06 12:44:17serhiy.storchakasetmessages: + msg181512
2013-02-06 10:36:52serhiy.storchakasetkeywords: + easy
nosy: + serhiy.storchaka

versions: + Python 3.4
2011-11-13 18:03:29rhettingersetmessages: + msg147564
2011-11-13 04:11:44terry.reedysettype: behavior
title: random.triangular error when low = mode -> random.triangular error when low = high=mode
components: + Library (Lib), - Extension Modules

keywords: + patch
nosy: + terry.reedy
versions: + Python 2.7, Python 3.2, Python 3.3, - Python 2.6
messages: + msg147548
stage: needs patch
2011-11-06 17:08:31rhettingersetassignee: rhettinger

nosy: + rhettinger
2011-11-06 12:22:22mark.dickinsonsetmessages: + msg147155
2011-11-06 11:27:39mark108setmessages: + msg147150
2011-11-06 11:09:21mark.dickinsonsetnosy: + mark.dickinson
messages: + msg147149
2011-11-06 10:56:56mark108create