-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cap the values that Beta.random can generate. #3924
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Luciano, LGTM! I just added some comments to correct typos -- will merge after this is fixed 💯
Thanks for the comments @AlexAndorra, I also fixed the failing test (at least locally). Should be ready to merge now. |
Codecov Report
@@ Coverage Diff @@
## master #3924 +/- ##
=======================================
Coverage 86.39% 86.40%
=======================================
Files 86 86
Lines 13722 13728 +6
=======================================
+ Hits 11855 11861 +6
Misses 1867 1867
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Luciano, looks all good, merging now :)
Closes #3898.
To prevent beta random variates to be equal to 0 or 1, we cap the values to
np.nextafter(0,1, dtype)
andnp.nextafter(1,0, dtype)
, which give the next floating point nearby zero and one respectively, depending on the precision of the floating point representation.Depending on what your PR does, here are a few things you might want to address in the description:
what are the (breaking) changes that this PR makes?
Samples returned by
Beta.random
can never be equal to 0 or 1.important background, or details about the implementation
are the changes—especially new features—covered by tests and docstrings?
Yes
consider adding/updating relevant example notebooks
right before it's ready to merge, mention the PR in the RELEASE-NOTES.md