Skip to content

BUG: random: Fix handling of a=0 for numpy.random.weibull.#12372

Merged
charris merged 2 commits intonumpy:masterfrom
WarrenWeckesser:bug-weibull0
Nov 12, 2018
Merged

BUG: random: Fix handling of a=0 for numpy.random.weibull.#12372
charris merged 2 commits intonumpy:masterfrom
WarrenWeckesser:bug-weibull0

Conversation

@WarrenWeckesser
Copy link
Member

Before this fix, np.random.weibull(a=0) often returned inf (and
in theory could have returned 1). It should only return 0.

Closes gh-12371.

Before this fix, np.random.weibull(a=0) often returned inf (and
in theory could have returned 1).  It should only return 0.

Closes numpygh-12371.

double rk_weibull(rk_state *state, double a)
{
if (a == 0.0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if a<0? The documentation says "Should be greater than 0". Is that accurate, or should it be changed to "Must be greater than 0" and we should error for negative values (raise an error in the pyx file)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Negative values are handled in RandomState.weibull in mtrand.pyx, so the C function is never called with negative values. For example,

In [39]: np.random.weibull(a=-0.0)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-39-ee6de7ad2cfe> in <module>()
----> 1 np.random.weibull(a=-0.0)

mtrand.pyx in mtrand.RandomState.weibull()

ValueError: a < 0

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess "Should be greater than zero" should have been changed to "Must be nonnegative" back when the code was changed to allow a=0. I can change that now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pushed an update to the docstring.

@rkern
Copy link
Member

rkern commented Nov 12, 2018

LGTM.

@WarrenWeckesser
Copy link
Member Author

I don't know what happened on that one test run on Travis CI that failed. Something killed the test script?

@charris
Copy link
Member

charris commented Nov 12, 2018

@WarrenWeckesser Travis times out pretty often these days, although I don't recall it dying in the middle of running the tests before. Seems like it should be unrelated to this PR in any case. Restarted just for the heck of it.

@charris charris merged commit 7c41164 into numpy:master Nov 12, 2018
@charris
Copy link
Member

charris commented Nov 12, 2018

Thanks Warren.

@WarrenWeckesser WarrenWeckesser deleted the bug-weibull0 branch November 13, 2018 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: random: np.random.weibull(a=0) sometimes returns inf.

4 participants