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.

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

Implement Distribution for Range(Inclusive) #821

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 72 additions & 2 deletions src/distributions/uniform.rs
Expand Up @@ -111,6 +111,7 @@
use std::time::Duration;
#[cfg(not(feature = "std"))]
use core::time::Duration;
use core::ops::{Range, RangeInclusive, RangeTo, RangeToInclusive};

use crate::Rng;
use crate::distributions::Distribution;
Expand Down Expand Up @@ -147,6 +148,9 @@ use packed_simd::*;
/// `Uniform::new(low, high)`, i.e., excluding `high`. In particular care must
/// be taken to ensure that rounding never results values `< low` or `>= high`.
///
/// Range expressions like `0..10` can be used as a `Uniform` distribution,
/// but are less efficient if multiple samples are taken.
///
/// # Example
///
/// ```
Expand Down Expand Up @@ -259,20 +263,71 @@ pub trait UniformSampler: Sized {
let uniform: Self = UniformSampler::new(low, high);
uniform.sample(rng)
}

/// Sample a single value uniformly from a range with inclusive lower bound
/// and inclusive upper bound `[low, high]`.
///
/// By default this is implemented using
/// `UniformSampler::new_inclusive(low, high).sample(rng)`. However, for
/// some types more optimal implementations for single usage may be provided
/// via this method.
/// Results may not be identical.
fn sample_single_inclusive<R: Rng + ?Sized, B1, B2>(low: B1, high: B2, rng: &mut R)
-> Self::X
where B1: SampleBorrow<Self::X> + Sized,
B2: SampleBorrow<Self::X> + Sized
{
let uniform: Self = UniformSampler::new_inclusive(low, high);
uniform.sample(rng)
Copy link
Member

Choose a reason for hiding this comment

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

It may make more sense to convert sample_single into sample_single_inclusive, then implement sample_single via sample_single_inclusive. At any rate, we should have an optimised implementation of this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

implement sample_single via sample_single_inclusive

Is there a way to do this without introducing an awkward where B2: From<u8> + Sub<B2> constraint?

}
}

impl<X: SampleUniform> From<::core::ops::Range<X>> for Uniform<X> {
impl<X: SampleUniform> From<Range<X>> for Uniform<X> {
fn from(r: ::core::ops::Range<X>) -> Uniform<X> {
Uniform::new(r.start, r.end)
}
}

impl<X: SampleUniform> From<::core::ops::RangeInclusive<X>> for Uniform<X> {
impl<X: SampleUniform> From<RangeInclusive<X>> for Uniform<X> {
fn from(r: ::core::ops::RangeInclusive<X>) -> Uniform<X> {
Uniform::new_inclusive(r.start(), r.end())
}
}

impl<T: SampleUniform> Distribution<T> for Range<T> {
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> T {
T::Sampler::sample_single(&self.start, &self.end, rng)
}
}

impl<T: SampleUniform> Distribution<T> for RangeInclusive<T> {
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> T {
T::Sampler::sample_single_inclusive(self.start(), self.end(), rng)
}
}

macro_rules! impl_distrib_range_to {
($($ty:ty),*) => {
$(
impl Distribution<$ty> for RangeTo<$ty> {
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> $ty {
<$ty as SampleUniform>::Sampler::sample_single(0, self.end, rng)
}
}
impl Distribution<$ty> for RangeToInclusive<$ty> {
fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> $ty {
<$ty as SampleUniform>::Sampler::sample_single_inclusive(0, self.end, rng)
}
}
)*
}
}

impl_distrib_range_to!(usize, u8, u16, u32, u64);
#[cfg(not(target_os = "emscripten"))]
impl_distrib_range_to!(u128);


/// Helper trait similar to [`Borrow`] but implemented
/// only for SampleUniform and references to SampleUniform in
/// order to resolve ambiguity issues.
Expand Down Expand Up @@ -1257,6 +1312,21 @@ mod tests {
assert_eq!(r.inner.scale, 5.0);
}

#[test]
fn test_std_range_distribution() {
let mut rng = crate::test::rng(474);
for _ in 0..100 {
let x = rng.sample(-5..5);
assert!(x >= -5 && x < 5);
let x = rng.sample(-5..=5);
assert!(x >= -5 && x <= 5);
let x = rng.sample(..10u8);
assert!(x < 10);
let x = rng.sample(..=10u8);
assert!(x <= 10);
}
}

#[test]
fn test_uniform_from_std_range_inclusive() {
let r = Uniform::from(2u32..=6);
Expand Down
5 changes: 3 additions & 2 deletions src/lib.rs
Expand Up @@ -227,9 +227,10 @@ pub trait Rng: RngCore {
/// use rand::distributions::Uniform;
///
/// let mut rng = thread_rng();
/// let x = rng.sample(Uniform::new(10u32, 15));
/// let x = rng.sample(10u32..15);
/// // Type annotation requires two types, the type and distribution; the
/// // distribution can be inferred.
/// // distribution can be inferred. `Uniform` is more efficient than the
/// // above range syntax if multiple samples are taken.
/// let y = rng.sample::<u16, _>(Uniform::new(10, 15));
/// ```
fn sample<T, D: Distribution<T>>(&mut self, distr: D) -> T {
Expand Down