-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[Numpy] Add sampling method for bernoulli #16638
Conversation
@reminisce |
Correctness for distribution has been briefly verified by hand.
|
Codecov Report
@@ Coverage Diff @@
## master #16638 +/- ##
==========================================
- Coverage 67% 66.94% -0.06%
==========================================
Files 264 271 +7
Lines 29507 29980 +473
Branches 4357 4440 +83
==========================================
+ Hits 19770 20071 +301
- Misses 8490 8637 +147
- Partials 1247 1272 +25
Continue to review full report at Codecov.
|
@sxjscience |
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.
LGTM except for 1 minor style problem. @sxjscience Any more comments?
Description
Native numpy does not support sampling directly from Bernoulli distribution, in order to do so, users have to use
np.random.binomial
and setn = 1
manually.However, I think a separate implementation for Bernoulli is worth consideration for the following reasons:
random.bernoulli
could be temporarily used as a ''work around'' for binomial sampling, see https://github.com/pytorch/pytorch/blob/071971476d7431a24e527bdc181981678055a95d/torch/distributions/binomial.py#L102 for more details. ( also, some discussions: torch.distributions.Binomial.sample() uses a massive amount of memory pytorch/pytorch#20343 )This pull request also fixes an existing bug in
np.random.uniform
, where scalar tensor would cause infer shape failure.Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments