| 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) *  |
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) * |
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) * |
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 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) * |
Date: 2011-11-13 18:03 |
| I've got this one. |
|
|
| msg181512 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-02-06 12:44 |
| Here is a patch. |
|
|
| msg181522 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-02-06 13:30 |
| > Here is a patch. Where? :-) |
|
|
| msg181525 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-02-06 14:27 |
| Now it is here. |
|
|
| msg181527 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-02-06 15:00 |
| Looks fine to me. Raymond: can this be applied? |
|
|
| msg181528 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2013-04-11 18:16 |
| Test for the issue_13355.patch is incorrect, please fix. |
|
|
| msg186678 - (view) |
Author: Roundup Robot (python-dev)  |
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) |
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) * |
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) * |
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) * |
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) * |
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) |
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) * |
Date: 2013-04-13 15:08 |
| Reverted. Sorry. |
|
|
| msg187836 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-04-26 07:24 |
| Raymond: have you had time to look at this yet? |
|
|
| msg187839 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2013-04-26 08:15 |
| Soonish |
|
|
| msg207792 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
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) * |
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) * |
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) * |
Date: 2014-05-24 00:43 |
| Attaching patch |
|
|
| msg219019 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-05-24 02:45 |
| So now triangular(10, 10, 20) will always return 10? |
|
|
| msg219033 - (view) |
Author: Stefan Krah (skrah) * |
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 "", line 1, in 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 "", line 1, in 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 "", line 1, in File "mtrand.pyx", line 3222, in mtrand.RandomState.triangular (numpy/random/mtrand/mtrand.c:13459) ValueError: left == right |
|
|
| msg219112 - (view) |
Author: Raymond Hettinger (rhettinger) * |
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) |
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) |
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) * |
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". |
|
|