BUG: Enforce high >= low on uniform number generators#17921
BUG: Enforce high >= low on uniform number generators#17921mattip merged 2 commits intonumpy:masterfrom
Conversation
|
I don't think this qualifies for an exception to the freezing of Please update the |
83e8452 to
c85593a
Compare
|
I agree with @rkern that we cannot change mtrand.pyx in this instance, and so the PR is limited to the new generator. It adds a check. An alternative would be to better explain that role of high low and make this the default behaviour. Something along the lines of Probably simpler to enforce though. Probably will also help to avoid hard to detect bugs by end-users. |
seberg
left a comment
There was a problem hiding this comment.
Thanks, I am happy to put this in then. Two small things:
- I guess it may be worth adding a release note, since it is a compatibility change in theory? (even if the new API isn't very old anyway)
- Just curious what happens/should happen for
low == high, and if we should test that.
There was a problem hiding this comment.
| assert_raises(ValueError, func, [[0, 1],[2, 3]], 2) | |
| assert_raises(ValueError, func, [[0, 1], [2, 3]], 2) |
c85593a to
0773972
Compare
That case is documented, if not tested. It always returns the constant |
|
It is also mathematically well defined using a Dirac delta.
…On Mon, Dec 7, 2020, 19:32 Robert Kern ***@***.***> wrote:
2. Just curious what happens/should happen for low == high, and if we
should test that.
That case is documented, if not tested. It always returns the constant low
(or equivalently, high).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#17921 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABKTSRNYGTT5OOCOWPGNS4DSTUUWJANCNFSM4UOCBDAA>
.
|
0773972 to
d53c497
Compare
Check that high is weakly larger than low and raise if now closes numpy#17905
This doesn't qualify for fixing under the NEP.
d53c497 to
a3bb19d
Compare
|
What about |
|
Thanks @bashtage |
|
With this change, it's now harder to generate a random number in the half-open interval |
That feels like a magic incarnation, at least given the lack of clear intent for this case doc. It doesn't seem to be that much larger to do what you wish. |
|
With floating point arithmetic, there are very no real guarantees that you'll actually get a half-open interval, on either side. |
Ah, I was misled by the docstring which seems to suggest the interval is truly half-open, only to remark in the notes that this is not guaranteed. Sounds like my concern isn't relevant, thanks for clarifying. |
|
There's definitely room for a |
Check that high is weakly larger than low and raise if now
closes #17905