Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Poisson: Fix undefined behavior and support f64 output #795

Merged
merged 9 commits into from
May 16, 2019
Merged

Poisson: Fix undefined behavior and support f64 output #795

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
31aad14
rand_distr: Check bounds before returning Poisson sample
vks May 15, 2019
c03e2c8
rand_distr: Add support for `f64` to `Poisson`
vks May 15, 2019
638b6be
rand_distr: Make Poisson generic
vks May 15, 2019
eec9bba
rand_distr: Add tests for f64 sampling from Poisson
vks May 15, 2019
90833ca
rand_distr: Add tests for f32 sampling from Poisson
vks May 15, 2019
2553cb5
rand_distr: Encourage inlining of Float methods
vks May 15, 2019
15b9a39
Address review feedback
vks May 15, 2019
84d89a7
Always calculate log_gamma using f64
vks May 15, 2019
ec99801
rand_distr: Inline Poisson sampling
vks May 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions rand_distr/src/poisson.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ where Standard: Distribution<N>
impl<N: Float> Distribution<N> for Poisson<N>
where Standard: Distribution<N>
{
#[inline]
Copy link
Member

Choose a reason for hiding this comment

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

This is a large function — I don't think #[inline] makes any sense here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is not #[inline(always)], so the compiler is still free to make that choice. It might make sense to inline it into the u64 sampling, so that some bound checks can be eliminated.

fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> N {
// using the algorithm from Numerical Recipes in C

Expand Down Expand Up @@ -124,6 +125,7 @@ where Standard: Distribution<N>
impl<N: Float> Distribution<u64> for Poisson<N>
where Standard: Distribution<N>
{
#[inline]
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> u64 {
vks marked this conversation as resolved.
Show resolved Hide resolved
let result: N = self.sample(rng);
result.to_u64().unwrap()
Expand Down