Skip to content

Deprecate Rng::gen_weighted_bool #308

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

Merged
merged 4 commits into from
Mar 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 1 deletion benches/generators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ const BYTES_LEN: usize = 1024;
use std::mem::size_of;
use test::{black_box, Bencher};

use rand::{RngCore, Rng, SeedableRng, NewRng, StdRng, OsRng, JitterRng, EntropyRng};
use rand::{RngCore, Rng, SeedableRng, NewRng};
use rand::{StdRng, SmallRng, OsRng, JitterRng, EntropyRng};
use rand::{XorShiftRng, Hc128Rng, IsaacRng, Isaac64Rng, ChaChaRng};
use rand::reseeding::ReseedingRng;
use rand::prng::hc128::Hc128Core;
Expand Down Expand Up @@ -37,6 +38,7 @@ gen_bytes!(gen_bytes_hc128, Hc128Rng::new());
gen_bytes!(gen_bytes_isaac, IsaacRng::new());
gen_bytes!(gen_bytes_isaac64, Isaac64Rng::new());
gen_bytes!(gen_bytes_std, StdRng::new());
gen_bytes!(gen_bytes_small, SmallRng::new());
gen_bytes!(gen_bytes_os, OsRng::new().unwrap());

macro_rules! gen_uint {
Expand All @@ -61,13 +63,15 @@ gen_uint!(gen_u32_hc128, u32, Hc128Rng::new());
gen_uint!(gen_u32_isaac, u32, IsaacRng::new());
gen_uint!(gen_u32_isaac64, u32, Isaac64Rng::new());
gen_uint!(gen_u32_std, u32, StdRng::new());
gen_uint!(gen_u32_small, u32, SmallRng::new());
gen_uint!(gen_u32_os, u32, OsRng::new().unwrap());

gen_uint!(gen_u64_xorshift, u64, XorShiftRng::new());
gen_uint!(gen_u64_hc128, u64, Hc128Rng::new());
gen_uint!(gen_u64_isaac, u64, IsaacRng::new());
gen_uint!(gen_u64_isaac64, u64, Isaac64Rng::new());
gen_uint!(gen_u64_std, u64, StdRng::new());
gen_uint!(gen_u64_small, u64, SmallRng::new());
gen_uint!(gen_u64_os, u64, OsRng::new().unwrap());

// Do not test JitterRng like the others by running it RAND_BENCH_N times per,
Expand Down
28 changes: 28 additions & 0 deletions benches/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,39 @@
extern crate test;
extern crate rand;

const RAND_BENCH_N: u64 = 1000;

use test::{black_box, Bencher};

use rand::{SeedableRng, SmallRng, Rng, thread_rng};
use rand::seq::*;

#[bench]
fn misc_gen_bool(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(&mut thread_rng()).unwrap();
b.iter(|| {
let mut accum = true;
for _ in 0..::RAND_BENCH_N {
accum ^= rng.gen_bool(0.18);
}
black_box(accum);
})
}

#[bench]
fn misc_gen_bool_var(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(&mut thread_rng()).unwrap();
b.iter(|| {
let mut p = 0.18;
let mut accum = true;
for _ in 0..::RAND_BENCH_N {
accum ^= rng.gen_bool(p);
p += 0.0001;
}
black_box(accum);
})
}

#[bench]
fn misc_shuffle_100(b: &mut Bencher) {
let mut rng = SmallRng::from_rng(&mut thread_rng()).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion src/distributions/binomial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl Distribution<u64> for Binomial {
if expected < 25.0 {
let mut lresult = 0.0;
for _ in 0 .. self.n {
if rng.gen::<f64>() < p {
if rng.gen_bool(p) {
lresult += 1.0;
}
}
Expand Down
35 changes: 33 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ pub trait Rng: RngCore {
/// # Example
///
/// ```rust
/// #[allow(deprecated)]
/// use rand::{thread_rng, Rng};
///
/// let mut rng = thread_rng();
Expand All @@ -532,11 +533,29 @@ pub trait Rng: RngCore {
/// // First meaningful use of `gen_weighted_bool`.
/// println!("{}", rng.gen_weighted_bool(3));
/// ```
#[deprecated(since="0.5.0", note="use gen_bool instead")]
fn gen_weighted_bool(&mut self, n: u32) -> bool {
// Short-circuit after `n <= 1` to avoid panic in `gen_range`
n <= 1 || self.gen_range(0, n) == 0
}

/// Return a bool with a probability `p` of being true.
///
/// # Example
///
/// ```rust
/// use rand::{thread_rng, Rng};
///
/// let mut rng = thread_rng();
/// println!("{}", rng.gen_bool(1.0 / 3.0));
/// ```
fn gen_bool(&mut self, p: f64) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

Please add a unit test, at the very least testing that gen_bool(1.0) doesn't panic, or better testing that both 1.0 and 0.0 produce the expected outputs a few times over.

assert!(p >= 0.0 && p <= 1.0);
// If `p` is constant, this will be evaluated at compile-time.
let p_int = (p * core::u32::MAX as f64) as u32;
self.gen::<u32>() <= p_int
}

/// Return an iterator of random characters from the set A-Z,a-z,0-9.
///
/// # Example
Expand Down Expand Up @@ -798,7 +817,7 @@ impl<R: SeedableRng> NewRng for R {
}
}

/// The standard RNG. The PRNG algorithm in `StdRng` is choosen to be efficient
/// The standard RNG. The PRNG algorithm in `StdRng` is chosen to be efficient
/// on the current platform, to be statistically strong and unpredictable
/// (meaning a cryptographically secure PRNG).
///
Expand Down Expand Up @@ -847,7 +866,7 @@ impl SeedableRng for StdRng {
}

/// An RNG recommended when small state, cheap initialization and good
/// performance are required. The PRNG algorithm in `SmallRng` is choosen to be
/// performance are required. The PRNG algorithm in `SmallRng` is chosen to be
/// efficient on the current platform, **without consideration for cryptography
Copy link
Member

Choose a reason for hiding this comment

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

Note that Xorshift has good next_u32 performance but not as good next_u64 performance as several other generators. Wait, something's wrong:

test gen_u32_xorshift      ... bench:       4,635 ns/iter (+/- 198) = 862 MB/s
test gen_u64_xorshift      ... bench:       2,840 ns/iter (+/- 93) = 2816 MB/s

Impossible that next_u64 is faster than next_u32. Anyway, be careful comparing benchmarks for gen_bool: I would imagine it most useful in heavy numerical simulators which would likely either use native 64-bit generators or buffered generators, i.e. a u32 may not be half the price of a u64.

Copy link
Contributor Author

@pitdicker pitdicker Mar 20, 2018

Choose a reason for hiding this comment

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

True, sometimes it will be about half the price, sometimes it just makes no difference.

And you are getting bitten again (I think) by the rust bug with multiple codegen units and benchmarks harness. Can you retry with export RUSTFLAGS="-C codegen-units=1"?

Copy link
Member

Choose a reason for hiding this comment

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

Interesting; didn't affect most tests (including xorshift bytes with around 900MB/s) but:

test gen_u32_xorshift      ... bench:       1,045 ns/iter (+/- 59) = 3827 MB/s

Copy link
Member

Choose a reason for hiding this comment

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

Actually, that was with another change calling black_box less frequently. Without that I get approx 3000 MB/s. The u64 results only change by about 50MB/s however.

/// or security**. The size of its state is much smaller than for `StdRng`.
///
Expand Down Expand Up @@ -890,10 +909,12 @@ impl SeedableRng for StdRng {
pub struct SmallRng(XorShiftRng);

impl RngCore for SmallRng {
#[inline(always)]
fn next_u32(&mut self) -> u32 {
self.0.next_u32()
}

#[inline(always)]
fn next_u64(&mut self) -> u64 {
self.0.next_u64()
}
Expand Down Expand Up @@ -1076,12 +1097,22 @@ mod test {
}

#[test]
#[allow(deprecated)]
fn test_gen_weighted_bool() {
let mut r = rng(104);
assert_eq!(r.gen_weighted_bool(0), true);
assert_eq!(r.gen_weighted_bool(1), true);
}

#[test]
fn test_gen_bool() {
let mut r = rng(105);
for _ in 0..5 {
assert_eq!(r.gen_bool(0.0), false);
assert_eq!(r.gen_bool(1.0), true);
}
}

#[test]
fn test_choose() {
let mut r = rng(107);
Expand Down