BUG: Enforce high >= low on uniform number generators

[bashtage]

Check that high is weakly larger than low and raise if now

closes #17905

[rkern]

I don't think this qualifies for an exception to the freezing of RandomState per NEP 19. I think this should be confined to Generator.

Please update the Notes section which still refers to the old behavior.

[bashtage]

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

The input values are not checked for order. The uniform is generated as low + (high - low) * random(), so that if
high < low, then the uniform is generated as if high and low are swaped.

Probably simpler to enforce though. Probably will also help to avoid hard to detect bugs by end-users.

[seberg]

Thanks, I am happy to put this in then. Two small things:

1. 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)
2. Just curious what happens/should happen for low == high, and if we should test that.

[seberg]

Suggested change

	assert_raises(ValueError, func, [[0, 1],[2, 3]], 2)
	assert_raises(ValueError, func, [[0, 1], [2, 3]], 2)

[rkern]

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

[bashtage]

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

[charris]

What about integers? There is #14333, which has other problems, but it seems somewhat related.

[bashtage]

What about integers? There is #14333, which has other problems, but it seems somewhat related.

#14333 is mostly a preference about the error message. This is about whether this is high > low is actually an error.

[mattip]

Thanks @bashtage

[eric-wieser]

With this change, it's now harder to generate a random number in the half-open interval (low, high], which I get the feeling was what passing the arguments in the "wrong" order did. Are we sure it wasn't better to support that use-case?

[bashtage]

With this change, it's now harder to generate a random number in the half-open interval (low, high], which I get the feeling was what passing the arguments in the "wrong" order did. Are we sure it wasn't better to support that use-case?

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.

-gen.uniform(-high, -low)

[rkern]

With floating point arithmetic, there are very no real guarantees that you'll actually get a half-open interval, on either side.

[eric-wieser]

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.

[rkern]

There's definitely room for a uniform-like method that fully controls whether the ends are open or closed, in any combination. But it should also enforce low <= high and control the open/closed-ness through other arguments.