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

Added a Distribution<usize> for Poisson #958

Closed
wants to merge 2 commits into from
Closed

Added a Distribution<usize> for Poisson #958

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
385a2d5
Added a Distribution<usize> for Poisson
CGMossa Apr 2, 2020
fca9cda
Added to_usize
CGMossa Apr 2, 2020
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
6 changes: 3 additions & 3 deletions rand_distr/src/poisson.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ where Standard: Distribution<N>
}

impl<N: Float> Distribution<u64> for Poisson<N>
where Standard: Distribution<N>
where Standard: Distribution<N>
{
#[inline]
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> u64 {
Expand All @@ -152,8 +152,8 @@ impl<N: Float> Distribution<usize> for Poisson<N>
{
#[inline]
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> usize {
let result: u64 = self.sample(rng);
result as usize
let result: N = self.sample(rng);
result.to_usize().unwrap()
Copy link
Collaborator

@vks vks May 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This may panic on 32-bit platforms. Maybe we should document that somewhere?

}
}

Expand Down
20 changes: 20 additions & 0 deletions rand_distr/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ pub trait Float:
/// Support converting to an unsigned integer.
fn to_u64(self) -> Option<u64>;

/// Support converting to an unsigned integer.
fn to_usize(self) -> Option<usize>;

/// Take the absolute value of self
fn abs(self) -> Self;
/// Take the largest integer less than or equal to self
Expand Down Expand Up @@ -82,6 +85,15 @@ impl Float for f32 {
}
}

#[inline]
fn to_usize(self) -> Option<usize> {
if self >= 0. && self <= ::core::usize::MAX as f32 {
Comment on lines +89 to +90
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 incorrect for 32-bit platforms. Check this out.

This isn't your fault; to_u64 for f64 makes the same mistake. Hmm, even the TryFrom in Rust 1.34 doesn't handle this conversion. I'm starting to hate this topic; seems simple but isn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh this issue. I don't know how to solve this aswell. The link didn't suggest a solution. I think this belongs in another PR, or something?

Copy link
Member

Choose a reason for hiding this comment

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

Yes: #973

Some(self as usize)
} else {
None
}
}

#[inline]
fn abs(self) -> Self {
self.abs()
Expand Down Expand Up @@ -145,6 +157,14 @@ impl Float for f64 {
None
}
}
#[inline]
fn to_usize(self) -> Option<usize> {
if self >= 0. && self <= ::core::usize::MAX as f64 {
Some(self as usize)
} else {
None
}
}

#[inline]
fn abs(self) -> Self {
Expand Down