From e783cb0537e492d59b5b6795740418bcc5fc5343 Mon Sep 17 00:00:00 2001 From: TheIronBorn Date: Mon, 19 Jul 2021 17:32:46 -0700 Subject: [PATCH 1/3] switch sample_single int to O'neill's modulo method & update gen_range_high benchmarks doc shortcutting sample_single int method mention which methods --- benches/distributions.rs | 14 +++++++------ src/distributions/uniform.rs | 38 +++++++++++++++++++----------------- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/benches/distributions.rs b/benches/distributions.rs index 7d8ac94c37b..2e0aaf2f3d2 100644 --- a/benches/distributions.rs +++ b/benches/distributions.rs @@ -218,12 +218,14 @@ gen_range_int!(gen_range_i64_low, i64, -1i64, 0); gen_range_int!(gen_range_i128_low, i128, -1i128, 0); // These were the initially tested ranges. They are likely to see fewer -// rejections than the low tests. -gen_range_int!(gen_range_i8_high, i8, -20i8, 100); -gen_range_int!(gen_range_i16_high, i16, -500i16, 2000); -gen_range_int!(gen_range_i32_high, i32, -200_000_000i32, 800_000_000); -gen_range_int!(gen_range_i64_high, i64, 3i64, 123_456_789_123); -gen_range_int!(gen_range_i128_high, i128, -12345678901234i128, 123_456_789_123_456_789); +// rejections than the low tests. The starting range here is +// `2^(N - 1) + 1`, the region with the highest rejection chance for +// modulo/bitmask/leading_zeros methods. +gen_range_int!(gen_range_i8_high, i8, i8::min_value(), 1); +gen_range_int!(gen_range_i16_high, i16, i16::min_value(), 1); +gen_range_int!(gen_range_i32_high, i32, i32::min_value(), 1); +gen_range_int!(gen_range_i64_high, i64, i64::min_value(), 1); +gen_range_int!(gen_range_i128_high, i128, i128::min_value(), 1); // construct and sample from a floating-point range macro_rules! gen_range_float { diff --git a/src/distributions/uniform.rs b/src/distributions/uniform.rs index 066ae0df1d0..b283f3a0b94 100644 --- a/src/distributions/uniform.rs +++ b/src/distributions/uniform.rs @@ -529,26 +529,28 @@ macro_rules! uniform_int_impl { return rng.gen(); } - let zone = if ::core::$unsigned::MAX <= ::core::u16::MAX as $unsigned { - // Using a modulus is faster than the approximation for - // i8 and i16. I suppose we trade the cost of one - // modulus for near-perfect branch prediction. - let unsigned_max: $u_large = ::core::$u_large::MAX; - let ints_to_reject = (unsigned_max - range + 1) % range; - unsigned_max - ints_to_reject - } else { - // conservative but fast approximation. `- 1` is necessary to allow the - // same comparison without bias. - (range << range.leading_zeros()).wrapping_sub(1) - }; - - loop { - let v: $u_large = rng.gen(); - let (hi, lo) = v.wmul(range); - if lo <= zone { - return low.wrapping_add(hi as $ty); + // we use the "Debiased Int Mult (t-opt, m-opt)" rejection sampling method + // described here https://www.pcg-random.org/posts/bounded-rands.html + // and here https://github.com/imneme/bounded-rands + + let (mut hi, mut lo) = rng.gen::<$u_large>().wmul(range); + // this shortcut works best with small ranges + if lo < range { + let mut threshold = range.wrapping_neg(); + // this shortcut works best with large ranges + if threshold >= range { + threshold -= range; + if threshold >= range { + threshold %= range; + } + } + while lo < threshold { + let (new_hi, new_lo) = rng.gen::<$u_large>().wmul(range); + hi = new_hi; + lo = new_lo; } } + low.wrapping_add(hi as $ty) } } }; From c5f0150671b2271a94a6dcbf2d415365e17a0bcc Mon Sep 17 00:00:00 2001 From: TheIronBorn Date: Wed, 28 Jul 2021 11:30:49 -0700 Subject: [PATCH 2/3] update stability test values --- src/seq/index.rs | 10 +++++----- src/seq/mod.rs | 40 ++++++++++++++++++++-------------------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/seq/index.rs b/src/seq/index.rs index ae36c323708..c35e0da782d 100644 --- a/src/seq/index.rs +++ b/src/seq/index.rs @@ -662,11 +662,11 @@ mod test { ); }; - do_test(10, 6, &[8, 0, 3, 5, 9, 6]); // floyd - do_test(25, 10, &[18, 15, 14, 9, 0, 13, 5, 24]); // floyd - do_test(300, 8, &[30, 283, 150, 1, 73, 13, 285, 35]); // floyd - do_test(300, 80, &[31, 289, 248, 154, 5, 78, 19, 286]); // inplace - do_test(300, 180, &[31, 289, 248, 154, 5, 78, 19, 286]); // inplace + do_test(10, 6, &[0, 9, 8, 6, 5, 4]); // floyd + do_test(25, 10, &[24, 1, 20, 16, 19, 22, 14, 9]); // floyd + do_test(300, 8, &[30, 283, 243, 150, 218, 240, 1, 189]); // floyd + do_test(300, 80, &[31, 289, 248, 154, 221, 243, 7, 192]); // inplace + do_test(300, 180, &[31, 289, 248, 154, 221, 243, 7, 192]); // inplace do_test(1_000_000, 8, &[ 103717, 963485, 826422, 509101, 736394, 807035, 5327, 632573, diff --git a/src/seq/mod.rs b/src/seq/mod.rs index 9eeb77749c9..917d9f1e66c 100644 --- a/src/seq/mod.rs +++ b/src/seq/mod.rs @@ -710,7 +710,7 @@ mod test { let mut nums = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]; assert_eq!(chars.choose(&mut r), Some(&'l')); - assert_eq!(nums.choose_mut(&mut r), Some(&mut 10)); + assert_eq!(nums.choose_mut(&mut r), Some(&mut 3)); #[cfg(feature = "alloc")] assert_eq!( @@ -718,21 +718,21 @@ mod test { .choose_multiple(&mut r, 8) .cloned() .collect::>(), - &['d', 'm', 'b', 'n', 'c', 'k', 'h', 'e'] + &['f', 'i', 'd', 'b', 'c', 'm', 'j', 'k'] ); #[cfg(feature = "alloc")] - assert_eq!(chars.choose_weighted(&mut r, |_| 1), Ok(&'f')); + assert_eq!(chars.choose_weighted(&mut r, |_| 1), Ok(&'l')); #[cfg(feature = "alloc")] - assert_eq!(nums.choose_weighted_mut(&mut r, |_| 1), Ok(&mut 5)); + assert_eq!(nums.choose_weighted_mut(&mut r, |_| 1), Ok(&mut 8)); let mut r = crate::test::rng(414); nums.shuffle(&mut r); - assert_eq!(nums, [9, 5, 3, 10, 7, 12, 8, 11, 6, 4, 0, 2, 1]); + assert_eq!(nums, [10, 11, 8, 7, 4, 6, 12, 5, 3, 0, 9, 2, 1]); nums = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]; let res = nums.partial_shuffle(&mut r, 6); - assert_eq!(res.0, &mut [7, 4, 8, 6, 9, 3]); - assert_eq!(res.1, &mut [0, 1, 2, 12, 11, 5, 10]); + assert_eq!(res.0, &mut [6, 10, 7, 2, 0, 8]); + assert_eq!(res.1, &mut [11, 1, 12, 3, 4, 5, 9]); } #[derive(Clone)] @@ -1122,7 +1122,7 @@ mod test { assert_eq!(choose([].iter().cloned()), None); assert_eq!(choose(0..100), Some(33)); - assert_eq!(choose(UnhintedIterator { iter: 0..100 }), Some(40)); + assert_eq!(choose(UnhintedIterator { iter: 0..100 }), Some(54)); assert_eq!( choose(ChunkHintedIterator { iter: 0..100, @@ -1130,7 +1130,7 @@ mod test { chunk_remaining: 32, hint_total_size: false, }), - Some(39) + Some(74) ); assert_eq!( choose(ChunkHintedIterator { @@ -1139,7 +1139,7 @@ mod test { chunk_remaining: 32, hint_total_size: true, }), - Some(39) + Some(74) ); assert_eq!( choose(WindowHintedIterator { @@ -1147,7 +1147,7 @@ mod test { window_size: 32, hint_total_size: false, }), - Some(90) + Some(34) ); assert_eq!( choose(WindowHintedIterator { @@ -1155,7 +1155,7 @@ mod test { window_size: 32, hint_total_size: true, }), - Some(90) + Some(34) ); } @@ -1167,8 +1167,8 @@ mod test { } assert_eq!(choose([].iter().cloned()), None); - assert_eq!(choose(0..100), Some(40)); - assert_eq!(choose(UnhintedIterator { iter: 0..100 }), Some(40)); + assert_eq!(choose(0..100), Some(54)); + assert_eq!(choose(UnhintedIterator { iter: 0..100 }), Some(54)); assert_eq!( choose(ChunkHintedIterator { iter: 0..100, @@ -1176,7 +1176,7 @@ mod test { chunk_remaining: 32, hint_total_size: false, }), - Some(40) + Some(54) ); assert_eq!( choose(ChunkHintedIterator { @@ -1185,7 +1185,7 @@ mod test { chunk_remaining: 32, hint_total_size: true, }), - Some(40) + Some(54) ); assert_eq!( choose(WindowHintedIterator { @@ -1193,7 +1193,7 @@ mod test { window_size: 32, hint_total_size: false, }), - Some(40) + Some(54) ); assert_eq!( choose(WindowHintedIterator { @@ -1201,7 +1201,7 @@ mod test { window_size: 32, hint_total_size: true, }), - Some(40) + Some(54) ); } @@ -1216,7 +1216,7 @@ mod test { do_test(0..4, &[0, 1, 2, 3]); do_test(0..8, &[0, 1, 2, 3, 4, 5, 6, 7]); - do_test(0..100, &[58, 78, 80, 92, 43, 8, 96, 7]); + do_test(0..100, &[77, 95, 38, 23, 25, 8, 58, 40]); #[cfg(feature = "alloc")] { @@ -1227,7 +1227,7 @@ mod test { do_test(0..4, &[0, 1, 2, 3]); do_test(0..8, &[0, 1, 2, 3, 4, 5, 6, 7]); - do_test(0..100, &[58, 78, 80, 92, 43, 8, 96, 7]); + do_test(0..100, &[77, 95, 38, 23, 25, 8, 58, 40]); } } From a69d6cc9e04daa5976de37fed2a233ba6fc7eb06 Mon Sep 17 00:00:00 2001 From: TheIronBorn Date: Fri, 6 Aug 2021 11:35:04 -0700 Subject: [PATCH 3/3] remove reference to old benchmarks --- benches/distributions.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/benches/distributions.rs b/benches/distributions.rs index 2e0aaf2f3d2..516c97f5cd1 100644 --- a/benches/distributions.rs +++ b/benches/distributions.rs @@ -217,9 +217,8 @@ gen_range_int!(gen_range_i32_low, i32, -1i32, 0); gen_range_int!(gen_range_i64_low, i64, -1i64, 0); gen_range_int!(gen_range_i128_low, i128, -1i128, 0); -// These were the initially tested ranges. They are likely to see fewer -// rejections than the low tests. The starting range here is -// `2^(N - 1) + 1`, the region with the highest rejection chance for +// These are likely to see fewer rejections than the low tests. The starting +// range here is `2^(N - 1) + 1`, the region with the highest rejection chance for // modulo/bitmask/leading_zeros methods. gen_range_int!(gen_range_i8_high, i8, i8::min_value(), 1); gen_range_int!(gen_range_i16_high, i16, i16::min_value(), 1);