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.

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.

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.

> 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.

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.

I've got this one.

Here is a patch.

> Here is a patch.

Where?  :-)

Now it is here.

Looks fine to me.  Raymond: can this be applied?

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.

> 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?

> What if low == high != mode?

Then we've got garbage in, garbage out;  that case doesn't worry me.

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.

> 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.

I'll look at the patch shortly.  At first glance, it looks over-engineered to me.

Added validation of input data. Check whether low <= mode <= high. If low == high return low as a result.

Test for the issue_13355.patch is incorrect, please fix.

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

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

Pushed. 
Yuriy Senko, thanks for patch. 
But, please, be more accurate with tests for another one.

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.

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.

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.

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

Reverted. Sorry.

Raymond:  have you had time to look at this yet?

Soonish

Raymond, could you please make a decision or delegate this issue to Mark, Terry, Andrew or me?

[Serhiy]
> Raymond, could you please make a decision

Yes, I will this week.

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.

Attaching patch

So now triangular(10, 10, 20) will always return 10?

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

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.

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

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

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".