gh-149221:Fix binomialvariate Function for random module#149222
gh-149221:Fix binomialvariate Function for random module#149222rhettinger merged 23 commits intopython:mainfrom
Conversation
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
…y.rst Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @rhettinger: please review the changes made to this pull request. |
skirpichev
left a comment
There was a problem hiding this comment.
I think it's not too hard to find a seed, that trigger an exception. Maybe issue worth a test?
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Documentation build overview
30 files changed ·
|
Removed unnecessary try-except block for log(0.0) handling.
|
This is all that is needed: |
What I originally had in mind was this: u = random()
if u == 0.0:
continue
y += _floor(_log2(u) / c) + 1No try-except, just a simple check and skip. @rhettinger |
|
I prefer the zero-cost try/except to not slow down the loop. |
Ok wait to change |
Handle ValueError in random number generation loop.
please review this @rhettinger. |
Co-authored-by: ByteFlow <fakeshadow1337@gmail.com>
Co-authored-by: ByteFlow <fakeshadow1337@gmail.com>
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @rhettinger: please review the changes made to this pull request. |
Fix:
Replace _log2(random()) with _log2(1.0 - random()). This matches the standard algorithm (1.0 - random() is never zero), produces the correct distribution, and eliminates the crash.
binomialvariatefunction returns wrong results #149221