-
-
Notifications
You must be signed in to change notification settings - Fork 445
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fix infinite loop in Binomial distribution #1325
Fix infinite loop in Binomial distribution #1325
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.
Looks good, thanks! There are some minor formatting issues, and it would be great to add the test case from #1324 as an integration test.
My case takes 5 minutes of computing to get there. I guess you can come up with seeds where this happens quickly. Essentially a u very close to 1 is required. |
Some formatting Co-authored-by: Vinzent Steinberg <Vinzent.Steinberg@gmail.com>
Yes, see my comment in #1324. I guess I could try to find a seed for the internal |
This test hangs without this PR. Maybe you can add it? #[test]
fn binomial_avoid_infinite_loop() {
let dist = Binomial::new(16000000, 3.1444753148558566e-10).unwrap();
let mut sum: u64 = 0;
let mut rng = crate::test::rng(742);
for _ in 0..100_000 {
sum = sum.wrapping_add(dist.sample(&mut rng));
}
assert_ne!(sum, 0);
} |
Great, thanks! |
See issue #1324
The solution is the same as in the GSL implementation. Just retry if you reach a certain number of iterations.