From 06d7019e4602a0e9254d4e6cf57800885359b5e7 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Mon, 9 Apr 2018 19:48:20 +0200 Subject: [PATCH 01/11] impl From for RangeInt --- src/distributions/range.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/distributions/range.rs b/src/distributions/range.rs index c36abdd21b2..e67a5e12aff 100644 --- a/src/distributions/range.rs +++ b/src/distributions/range.rs @@ -314,6 +314,12 @@ macro_rules! range_int_impl { } } } + + impl From<::core::ops::Range<$ty>> for RangeInt<$ty> { + fn from(r: ::core::ops::Range<$ty>) -> RangeInt<$ty> { + RangeInt::<$ty>::new(r.start, r.end) + } + } } } @@ -469,7 +475,7 @@ range_float_impl! { f64, 64 - 52, next_u64 } #[cfg(test)] mod tests { use Rng; - use distributions::range::{Range, RangeImpl, RangeFloat, SampleRange}; + use distributions::range::{Range, RangeImpl, RangeInt, RangeFloat, SampleRange}; #[should_panic] #[test] @@ -578,4 +584,11 @@ mod tests { assert!(low <= x && x < high); } } + + #[test] + fn test_from_std_range() { + let r = RangeInt::from(2..7); + assert_eq!(r.low, 2); + assert_eq!(r.range, 5); + } } From f3495ad0f83ea2fec581d8c2647fafdb118a6296 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Mon, 9 Apr 2018 20:24:13 +0200 Subject: [PATCH 02/11] impl From for Range --- src/distributions/range.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/distributions/range.rs b/src/distributions/range.rs index e67a5e12aff..4ea6ca76539 100644 --- a/src/distributions/range.rs +++ b/src/distributions/range.rs @@ -320,6 +320,13 @@ macro_rules! range_int_impl { RangeInt::<$ty>::new(r.start, r.end) } } + + impl From<::core::ops::Range<$ty>> for Range<$ty> + { + fn from(r: ::core::ops::Range<$ty>) -> Range<$ty> { + Range { inner: <$ty as SampleRange>::Impl::from(r) } + } + } } } @@ -586,9 +593,16 @@ mod tests { } #[test] - fn test_from_std_range() { + fn test_rangeint_from_std_range() { let r = RangeInt::from(2..7); assert_eq!(r.low, 2); assert_eq!(r.range, 5); } + + #[test] + fn test_range_from_std_range() { + let r = Range::::from(2..7); + assert_eq!(r.inner.low, 2u32); + assert_eq!(r.inner.range, 5u32); + } } From ef9a74241069ebc4f2c74a72508b7baa3399d8eb Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Mon, 9 Apr 2018 20:26:30 +0200 Subject: [PATCH 03/11] Change `Range` doctest to use std::ops::Range --- src/distributions/range.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/distributions/range.rs b/src/distributions/range.rs index 4ea6ca76539..fd3a0d499cd 100644 --- a/src/distributions/range.rs +++ b/src/distributions/range.rs @@ -42,7 +42,7 @@ use distributions::float::IntoFloat; /// use rand::distributions::{Distribution, Range}; /// /// fn main() { -/// let between = Range::new(10, 10000); +/// let between = Range::from(10..10000); /// let mut rng = rand::thread_rng(); /// let mut sum = 0; /// for _ in 0..1000 { From 9bc2c2e1d7d5cada7de7b0c6b32277a4ea359e22 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Tue, 10 Apr 2018 13:33:20 +0200 Subject: [PATCH 04/11] impl From for RangeFloat --- src/distributions/range.rs | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/distributions/range.rs b/src/distributions/range.rs index fd3a0d499cd..71d3ad7ace1 100644 --- a/src/distributions/range.rs +++ b/src/distributions/range.rs @@ -320,13 +320,15 @@ macro_rules! range_int_impl { RangeInt::<$ty>::new(r.start, r.end) } } + } +} - impl From<::core::ops::Range<$ty>> for Range<$ty> - { - fn from(r: ::core::ops::Range<$ty>) -> Range<$ty> { - Range { inner: <$ty as SampleRange>::Impl::from(r) } - } - } +impl From<::core::ops::Range> for Range + where X: SampleRange, + ::Impl: From<::core::ops::Range> +{ + fn from(r: ::core::ops::Range) -> Range { + Range { inner: ::Impl::from(r) } } } @@ -472,6 +474,12 @@ macro_rules! range_float_impl { value1_2 * self.scale + self.offset } } + + impl From<::core::ops::Range<$ty>> for RangeFloat<$ty> { + fn from(r: ::core::ops::Range<$ty>) -> RangeFloat<$ty> { + RangeFloat::<$ty>::new(r.start, r.end) + } + } } } @@ -597,12 +605,18 @@ mod tests { let r = RangeInt::from(2..7); assert_eq!(r.low, 2); assert_eq!(r.range, 5); + let r = RangeFloat::from(2.0..7.0); + assert_eq!(r.offset, -3.0); + assert_eq!(r.scale, 5.0); } #[test] fn test_range_from_std_range() { - let r = Range::::from(2..7); - assert_eq!(r.inner.low, 2u32); - assert_eq!(r.inner.range, 5u32); + let r = Range::from(2u32..7); + assert_eq!(r.inner.low, 2); + assert_eq!(r.inner.range, 5); + let r = Range::from(2.0f64..7.0); + assert_eq!(r.inner.offset, -3.0); + assert_eq!(r.inner.scale, 5.0); } } From 13a639c01c58a46445eeb14c6ad48f4186418f35 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Tue, 10 Apr 2018 13:39:47 +0200 Subject: [PATCH 05/11] Use `mul_add` --- src/distributions/range.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/distributions/range.rs b/src/distributions/range.rs index 71d3ad7ace1..67d01d378de 100644 --- a/src/distributions/range.rs +++ b/src/distributions/range.rs @@ -469,9 +469,7 @@ macro_rules! range_float_impl { // Generate a value in the range [1, 2) let value1_2 = (rng.$next_u() >> $bits_to_discard) .into_float_with_exponent(0); - // Doing multiply before addition allows some architectures to - // use a single instruction. - value1_2 * self.scale + self.offset + value1_2.mul_add(self.scale, self.offset) } } From c97d6f4507ff143cdec3e363aa5dd216faf842bf Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Wed, 11 Apr 2018 15:50:27 +0200 Subject: [PATCH 06/11] Don't use `mul_add` It doesn't work with `no_std` and it is sometimes slower. This was documented in a comment. --- src/distributions/range.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/distributions/range.rs b/src/distributions/range.rs index 67d01d378de..02c1e2830e4 100644 --- a/src/distributions/range.rs +++ b/src/distributions/range.rs @@ -469,7 +469,10 @@ macro_rules! range_float_impl { // Generate a value in the range [1, 2) let value1_2 = (rng.$next_u() >> $bits_to_discard) .into_float_with_exponent(0); - value1_2.mul_add(self.scale, self.offset) + // We don't use `f64::mul_add`, because it is not available with + // `no_std`. Furthermore, it is slower for some targets (but + // faster for others). + value1_2 * self.scale + self.offset } } From 869a367b5603cbcc8d702f48e653bbf28919cf72 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Wed, 11 Apr 2018 19:00:07 +0200 Subject: [PATCH 07/11] Make From impl more generic --- src/distributions/range.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/distributions/range.rs b/src/distributions/range.rs index 02c1e2830e4..57d7cfa0311 100644 --- a/src/distributions/range.rs +++ b/src/distributions/range.rs @@ -323,12 +323,9 @@ macro_rules! range_int_impl { } } -impl From<::core::ops::Range> for Range - where X: SampleRange, - ::Impl: From<::core::ops::Range> -{ +impl From<::core::ops::Range> for Range { fn from(r: ::core::ops::Range) -> Range { - Range { inner: ::Impl::from(r) } + Range::new(r.start, r.end) } } From ecec2b6894026ba964bd463cb5d3dea5701c8d5e Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Thu, 12 Apr 2018 19:18:34 +0200 Subject: [PATCH 08/11] Add comment about optimal operation order --- src/distributions/range.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/distributions/range.rs b/src/distributions/range.rs index 57d7cfa0311..43732297064 100644 --- a/src/distributions/range.rs +++ b/src/distributions/range.rs @@ -468,7 +468,9 @@ macro_rules! range_float_impl { .into_float_with_exponent(0); // We don't use `f64::mul_add`, because it is not available with // `no_std`. Furthermore, it is slower for some targets (but - // faster for others). + // faster for others). However, the order of multiplication and + // addition is important, because on some platforms (e.g. ARM) + // it will be optimized to a single (non-FMA) instruction. value1_2 * self.scale + self.offset } } From 6d5eda71092e14d1ab45cf6244cc46d4a7500baa Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Thu, 12 Apr 2018 19:28:57 +0200 Subject: [PATCH 09/11] Document that `RangeInt` and `RangeFloat` should not be used directly --- src/distributions/range.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/distributions/range.rs b/src/distributions/range.rs index 43732297064..a77fc77276f 100644 --- a/src/distributions/range.rs +++ b/src/distributions/range.rs @@ -173,6 +173,8 @@ pub trait RangeImpl: Sized { } /// Implementation of `RangeImpl` for integer types. +/// +/// This type should not be used directly, use `Range` instead. #[derive(Clone, Copy, Debug)] pub struct RangeInt { low: X, @@ -432,6 +434,8 @@ wmul_impl_usize! { u64 } /// Implementation of `RangeImpl` for float types. +/// +/// This type should not be used directly, use `Range` instead. #[derive(Clone, Copy, Debug)] pub struct RangeFloat { scale: X, From 6a93182bee725ceb3d6c5c5ee1daa32424e9b948 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Thu, 12 Apr 2018 19:31:16 +0200 Subject: [PATCH 10/11] Don't `impl From for Range{Int,Float}` They should not be used directly anyway. --- src/distributions/range.rs | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/src/distributions/range.rs b/src/distributions/range.rs index a77fc77276f..4f491b8a079 100644 --- a/src/distributions/range.rs +++ b/src/distributions/range.rs @@ -316,12 +316,6 @@ macro_rules! range_int_impl { } } } - - impl From<::core::ops::Range<$ty>> for RangeInt<$ty> { - fn from(r: ::core::ops::Range<$ty>) -> RangeInt<$ty> { - RangeInt::<$ty>::new(r.start, r.end) - } - } } } @@ -478,12 +472,6 @@ macro_rules! range_float_impl { value1_2 * self.scale + self.offset } } - - impl From<::core::ops::Range<$ty>> for RangeFloat<$ty> { - fn from(r: ::core::ops::Range<$ty>) -> RangeFloat<$ty> { - RangeFloat::<$ty>::new(r.start, r.end) - } - } } } @@ -604,16 +592,6 @@ mod tests { } } - #[test] - fn test_rangeint_from_std_range() { - let r = RangeInt::from(2..7); - assert_eq!(r.low, 2); - assert_eq!(r.range, 5); - let r = RangeFloat::from(2.0..7.0); - assert_eq!(r.offset, -3.0); - assert_eq!(r.scale, 5.0); - } - #[test] fn test_range_from_std_range() { let r = Range::from(2u32..7); From 37567205614edd1fa690309de494476f8e20fee2 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Thu, 12 Apr 2018 19:47:51 +0200 Subject: [PATCH 11/11] Clarify comment --- src/distributions/range.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/distributions/range.rs b/src/distributions/range.rs index 4f491b8a079..626f0bc5199 100644 --- a/src/distributions/range.rs +++ b/src/distributions/range.rs @@ -174,7 +174,8 @@ pub trait RangeImpl: Sized { /// Implementation of `RangeImpl` for integer types. /// -/// This type should not be used directly, use `Range` instead. +/// Unless you are implementing `RangeImpl` for your own type, this type should +/// not be used directly, use `Range` instead. #[derive(Clone, Copy, Debug)] pub struct RangeInt { low: X, @@ -429,7 +430,8 @@ wmul_impl_usize! { u64 } /// Implementation of `RangeImpl` for float types. /// -/// This type should not be used directly, use `Range` instead. +/// Unless you are implementing `RangeImpl` for your own type, this type should +/// not be used directly, use `Range` instead. #[derive(Clone, Copy, Debug)] pub struct RangeFloat { scale: X,