From f9bce609b2d3c16c702dc708dd05aff77d055ee2 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Fri, 22 Apr 2022 22:17:10 +0200 Subject: [PATCH 1/7] Forbid unsafe code in crates without unsafe code This helps tools like `cargo geiger`. --- rand_chacha/src/lib.rs | 1 + rand_distr/src/lib.rs | 1 + rand_pcg/src/lib.rs | 1 + 3 files changed, 3 insertions(+) diff --git a/rand_chacha/src/lib.rs b/rand_chacha/src/lib.rs index 24125b45e10..f4b526b8f64 100644 --- a/rand_chacha/src/lib.rs +++ b/rand_chacha/src/lib.rs @@ -13,6 +13,7 @@ html_favicon_url = "https://www.rust-lang.org/favicon.ico", html_root_url = "https://rust-random.github.io/rand/" )] +#![forbid(unsafe_code)] #![deny(missing_docs)] #![deny(missing_debug_implementations)] #![doc(test(attr(allow(unused_variables), deny(warnings))))] diff --git a/rand_distr/src/lib.rs b/rand_distr/src/lib.rs index 6d8d81bd2f3..c8fd298171d 100644 --- a/rand_distr/src/lib.rs +++ b/rand_distr/src/lib.rs @@ -11,6 +11,7 @@ html_favicon_url = "https://www.rust-lang.org/favicon.ico", html_root_url = "https://rust-random.github.io/rand/" )] +#![forbid(unsafe_code)] #![deny(missing_docs)] #![deny(missing_debug_implementations)] #![allow( diff --git a/rand_pcg/src/lib.rs b/rand_pcg/src/lib.rs index 9d0209d14fe..341313954e7 100644 --- a/rand_pcg/src/lib.rs +++ b/rand_pcg/src/lib.rs @@ -33,6 +33,7 @@ html_favicon_url = "https://www.rust-lang.org/favicon.ico", html_root_url = "https://rust-random.github.io/rand/" )] +#![forbid(unsafe_code)] #![deny(missing_docs)] #![deny(missing_debug_implementations)] #![no_std] From e40f00a2f4b3ca1b2b9b215d425fb41f7f6d22c0 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Fri, 22 Apr 2022 22:40:52 +0200 Subject: [PATCH 2/7] Make `Uniform` constructors return a result - This is a breaking change. - The new error type had to be made public, otherwise `Uniform` could not be extended for user-defined types by implementing `UniformSampler`. - `rand_distr` was updated accordingly. - Also forbid unsafe code for crates where none is used. Fixes #1195, #1211. --- CHANGELOG.md | 5 + Cargo.toml | 2 +- examples/monte-carlo.rs | 2 +- examples/monty-hall.rs | 2 +- rand_distr/CHANGELOG.md | 1 + rand_distr/Cargo.toml | 4 +- rand_distr/src/binomial.rs | 4 +- rand_distr/src/hypergeometric.rs | 2 +- rand_distr/src/unit_ball.rs | 2 +- rand_distr/src/unit_circle.rs | 2 +- rand_distr/src/unit_disc.rs | 2 +- rand_distr/src/unit_sphere.rs | 2 +- rand_distr/src/weighted_alias.rs | 6 +- src/distributions/distribution.rs | 7 +- src/distributions/other.rs | 4 +- src/distributions/slice.rs | 2 +- src/distributions/uniform.rs | 306 +++++++++++++++------------- src/distributions/weighted_index.rs | 4 +- src/rng.rs | 6 +- src/seq/index.rs | 2 +- 20 files changed, 199 insertions(+), 168 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0872af6d39..12bfd8e90c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ A [separate changelog is kept for rand_core](rand_core/CHANGELOG.md). You may also find the [Upgrade Guide](https://rust-random.github.io/book/update.html) useful. +## [0.9.0] - unreleased +### Distributions +- `{Uniform, UniformSampler}::{new, new_inclusive}` return a `Result` (instead of potentially panicking) +- `Uniform` implements `TryFrom` instead of `From` for ranges + ## [0.8.5] - 2021-08-20 ### Fixes - Fix build on non-32/64-bit architectures (#1144) diff --git a/Cargo.toml b/Cargo.toml index 98ba373c68f..bbdda34baeb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rand" -version = "0.8.5" +version = "0.9.0" authors = ["The Rand Project Developers", "The Rust Project Developers"] license = "MIT OR Apache-2.0" readme = "README.md" diff --git a/examples/monte-carlo.rs b/examples/monte-carlo.rs index 6cc9f4e142a..a72cc1e9f47 100644 --- a/examples/monte-carlo.rs +++ b/examples/monte-carlo.rs @@ -29,7 +29,7 @@ use rand::distributions::{Distribution, Uniform}; fn main() { - let range = Uniform::new(-1.0f64, 1.0); + let range = Uniform::new(-1.0f64, 1.0).unwrap(); let mut rng = rand::thread_rng(); let total = 1_000_000; diff --git a/examples/monty-hall.rs b/examples/monty-hall.rs index 2a3b63d8df3..23e11178969 100644 --- a/examples/monty-hall.rs +++ b/examples/monty-hall.rs @@ -80,7 +80,7 @@ fn main() { let num_simulations = 10000; let mut rng = rand::thread_rng(); - let random_door = Uniform::new(0u32, 3); + let random_door = Uniform::new(0u32, 3).unwrap(); let (mut switch_wins, mut switch_losses) = (0, 0); let (mut keep_wins, mut keep_losses) = (0, 0); diff --git a/rand_distr/CHANGELOG.md b/rand_distr/CHANGELOG.md index 66389d670bf..13dac1c8a4d 100644 --- a/rand_distr/CHANGELOG.md +++ b/rand_distr/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.5.0] - unreleased - Remove unused fields from `Gamma`, `NormalInverseGaussian` and `Zipf` distributions (#1184) This breaks serialization compatibility with older versions. +- Upgrade Rand ## [0.4.3] - 2021-12-30 - Fix `no_std` build (#1208) diff --git a/rand_distr/Cargo.toml b/rand_distr/Cargo.toml index 32a5fcaf5ae..edea724a61c 100644 --- a/rand_distr/Cargo.toml +++ b/rand_distr/Cargo.toml @@ -23,14 +23,14 @@ std_math = ["num-traits/std"] serde1 = ["serde", "rand/serde1"] [dependencies] -rand = { path = "..", version = "0.8.0", default-features = false } +rand = { path = "..", version = "0.9.0", default-features = false } num-traits = { version = "0.2", default-features = false, features = ["libm"] } serde = { version = "1.0.103", features = ["derive"], optional = true } [dev-dependencies] rand_pcg = { version = "0.3.0", path = "../rand_pcg" } # For inline examples -rand = { path = "..", version = "0.8.0", default-features = false, features = ["std_rng", "std", "small_rng"] } +rand = { path = "..", version = "0.9.0", default-features = false, features = ["std_rng", "std", "small_rng"] } # Histogram implementation for testing uniformity average = { version = "0.13", features = [ "std" ] } # Special functions for testing distributions diff --git a/rand_distr/src/binomial.rs b/rand_distr/src/binomial.rs index 6dbf7ab7494..0f49806aba1 100644 --- a/rand_distr/src/binomial.rs +++ b/rand_distr/src/binomial.rs @@ -163,8 +163,8 @@ impl Distribution for Binomial { // return value let mut y: i64; - let gen_u = Uniform::new(0., p4); - let gen_v = Uniform::new(0., 1.); + let gen_u = Uniform::new(0., p4).unwrap(); + let gen_v = Uniform::new(0., 1.).unwrap(); loop { // Step 1: Generate `u` for selecting the region. If region 1 is diff --git a/rand_distr/src/hypergeometric.rs b/rand_distr/src/hypergeometric.rs index 4761450360d..a42a572d8d1 100644 --- a/rand_distr/src/hypergeometric.rs +++ b/rand_distr/src/hypergeometric.rs @@ -251,7 +251,7 @@ impl Distribution for Hypergeometric { x }, RejectionAcceptance { m, a, lambda_l, lambda_r, x_l, x_r, p1, p2, p3 } => { - let distr_region_select = Uniform::new(0.0, p3); + let distr_region_select = Uniform::new(0.0, p3).unwrap(); loop { let (y, v) = loop { let u = distr_region_select.sample(rng); diff --git a/rand_distr/src/unit_ball.rs b/rand_distr/src/unit_ball.rs index 8a4b4fbf3d1..4d29612597f 100644 --- a/rand_distr/src/unit_ball.rs +++ b/rand_distr/src/unit_ball.rs @@ -31,7 +31,7 @@ pub struct UnitBall; impl Distribution<[F; 3]> for UnitBall { #[inline] fn sample(&self, rng: &mut R) -> [F; 3] { - let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()); + let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()).unwrap(); let mut x1; let mut x2; let mut x3; diff --git a/rand_distr/src/unit_circle.rs b/rand_distr/src/unit_circle.rs index 24a06f3f4de..f3dbe757aa9 100644 --- a/rand_distr/src/unit_circle.rs +++ b/rand_distr/src/unit_circle.rs @@ -35,7 +35,7 @@ pub struct UnitCircle; impl Distribution<[F; 2]> for UnitCircle { #[inline] fn sample(&self, rng: &mut R) -> [F; 2] { - let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()); + let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()).unwrap(); let mut x1; let mut x2; let mut sum; diff --git a/rand_distr/src/unit_disc.rs b/rand_distr/src/unit_disc.rs index 937c1d01b84..5004217d5b7 100644 --- a/rand_distr/src/unit_disc.rs +++ b/rand_distr/src/unit_disc.rs @@ -30,7 +30,7 @@ pub struct UnitDisc; impl Distribution<[F; 2]> for UnitDisc { #[inline] fn sample(&self, rng: &mut R) -> [F; 2] { - let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()); + let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()).unwrap(); let mut x1; let mut x2; loop { diff --git a/rand_distr/src/unit_sphere.rs b/rand_distr/src/unit_sphere.rs index 2b299239f49..632275e3327 100644 --- a/rand_distr/src/unit_sphere.rs +++ b/rand_distr/src/unit_sphere.rs @@ -34,7 +34,7 @@ pub struct UnitSphere; impl Distribution<[F; 3]> for UnitSphere { #[inline] fn sample(&self, rng: &mut R) -> [F; 3] { - let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()); + let uniform = Uniform::new(F::from(-1.).unwrap(), F::from(1.).unwrap()).unwrap(); loop { let (x1, x2) = (uniform.sample(rng), uniform.sample(rng)); let sum = x1 * x1 + x2 * x2; diff --git a/rand_distr/src/weighted_alias.rs b/rand_distr/src/weighted_alias.rs index 582a4dd9ba8..170de80c4a5 100644 --- a/rand_distr/src/weighted_alias.rs +++ b/rand_distr/src/weighted_alias.rs @@ -221,8 +221,8 @@ impl WeightedAliasIndex { // Prepare distributions for sampling. Creating them beforehand improves // sampling performance. - let uniform_index = Uniform::new(0, n); - let uniform_within_weight_sum = Uniform::new(W::ZERO, weight_sum); + let uniform_index = Uniform::new(0, n).unwrap(); + let uniform_within_weight_sum = Uniform::new(W::ZERO, weight_sum).unwrap(); Ok(Self { aliases: aliases.aliases, @@ -458,7 +458,7 @@ mod test { let random_weight_distribution = Uniform::new_inclusive( W::ZERO, W::MAX / W::try_from_u32_lossy(NUM_WEIGHTS).unwrap(), - ); + ).unwrap(); for _ in 0..NUM_WEIGHTS { weights.push(rng.sample(&random_weight_distribution)); } diff --git a/src/distributions/distribution.rs b/src/distributions/distribution.rs index c5cf6a607b4..86f4c44112a 100644 --- a/src/distributions/distribution.rs +++ b/src/distributions/distribution.rs @@ -64,7 +64,7 @@ pub trait Distribution { /// .collect(); /// /// // Dice-rolling: - /// let die_range = Uniform::new_inclusive(1, 6); + /// let die_range = Uniform::new_inclusive(1, 6).unwrap(); /// let mut roll_die = die_range.sample_iter(&mut rng); /// while roll_die.next().unwrap() != 6 { /// println!("Not a 6; rolling again!"); @@ -93,7 +93,7 @@ pub trait Distribution { /// /// let mut rng = thread_rng(); /// - /// let die = Uniform::new_inclusive(1, 6); + /// let die = Uniform::new_inclusive(1, 6).unwrap(); /// let even_number = die.map(|num| num % 2 == 0); /// while !even_number.sample(&mut rng) { /// println!("Still odd; rolling again!"); @@ -227,7 +227,7 @@ mod tests { #[test] fn test_distributions_map() { - let dist = Uniform::new_inclusive(0, 5).map(|val| val + 15); + let dist = Uniform::new_inclusive(0, 5).unwrap().map(|val| val + 15); let mut rng = crate::test::rng(212); let val = dist.sample(&mut rng); @@ -240,6 +240,7 @@ mod tests { rng: &mut R, ) -> impl Iterator + '_ { Uniform::new_inclusive(1, 6) + .unwrap() .sample_iter(rng) .filter(|x| *x != 5) .take(10) diff --git a/src/distributions/other.rs b/src/distributions/other.rs index 03802a76d5f..fd5ce4ea725 100644 --- a/src/distributions/other.rs +++ b/src/distributions/other.rs @@ -80,9 +80,9 @@ impl Distribution for Standard { // reserved for surrogates. This is the size of that gap. const GAP_SIZE: u32 = 0xDFFF - 0xD800 + 1; - // Uniform::new(0, 0x11_0000 - GAP_SIZE) can also be used but it + // Uniform::new(0, 0x11_0000 - GAP_SIZE) can also be used, but it // seemed slower. - let range = Uniform::new(GAP_SIZE, 0x11_0000); + let range = Uniform::new(GAP_SIZE, 0x11_0000).unwrap(); let mut n = range.sample(rng); if n <= 0xDFFF { diff --git a/src/distributions/slice.rs b/src/distributions/slice.rs index 3302deb2a40..398cad18b2c 100644 --- a/src/distributions/slice.rs +++ b/src/distributions/slice.rs @@ -75,7 +75,7 @@ impl<'a, T> Slice<'a, T> { 0 => Err(EmptySlice), len => Ok(Self { slice, - range: Uniform::new(0, len), + range: Uniform::new(0, len).unwrap(), }), } } diff --git a/src/distributions/uniform.rs b/src/distributions/uniform.rs index 261357b2456..4be7a5c5e67 100644 --- a/src/distributions/uniform.rs +++ b/src/distributions/uniform.rs @@ -10,9 +10,8 @@ //! A distribution uniformly sampling numbers within a given range. //! //! [`Uniform`] is the standard distribution to sample uniformly from a range; -//! e.g. `Uniform::new_inclusive(1, 6)` can sample integers from 1 to 6, like a -//! standard die. [`Rng::gen_range`] supports any type supported by -//! [`Uniform`]. +//! e.g. `Uniform::new_inclusive(1, 6).unwrap()` can sample integers from 1 to 6, like a +//! standard die. [`Rng::gen_range`] supports any type supported by [`Uniform`]. //! //! This distribution is provided with support for several primitive types //! (all integer and floating-point types) as well as [`std::time::Duration`], @@ -31,7 +30,7 @@ //! use rand::distributions::Uniform; //! //! let mut rng = thread_rng(); -//! let side = Uniform::new(-10.0, 10.0); +//! let side = Uniform::new(-10.0, 10.0).unwrap(); //! //! // sample between 1 and 10 points //! for _ in 0..rng.gen_range(1..=10) { @@ -49,11 +48,11 @@ //! //! At a minimum, the back-end needs to store any parameters needed for sampling //! (e.g. the target range) and implement `new`, `new_inclusive` and `sample`. -//! Those methods should include an assert to check the range is valid (i.e. +//! Those methods should include an assertion to check the range is valid (i.e. //! `low < high`). The example below merely wraps another back-end. //! //! The `new`, `new_inclusive` and `sample_single` functions use arguments of -//! type SampleBorrow in order to support passing in values by reference or +//! type SampleBorrow to support passing in values by reference or //! by value. In the implementation of these functions, you can choose to //! simply use the reference returned by [`SampleBorrow::borrow`], or you can choose //! to copy or clone the value, whatever is appropriate for your type. @@ -61,7 +60,7 @@ //! ``` //! use rand::prelude::*; //! use rand::distributions::uniform::{Uniform, SampleUniform, -//! UniformSampler, UniformFloat, SampleBorrow}; +//! UniformSampler, UniformFloat, SampleBorrow, Error}; //! //! struct MyF32(f32); //! @@ -70,20 +69,18 @@ //! //! impl UniformSampler for UniformMyF32 { //! type X = MyF32; -//! fn new(low: B1, high: B2) -> Self +//! +//! fn new(low: B1, high: B2) -> Result //! where B1: SampleBorrow + Sized, //! B2: SampleBorrow + Sized //! { -//! UniformMyF32(UniformFloat::::new(low.borrow().0, high.borrow().0)) +//! UniformFloat::::new(low.borrow().0, high.borrow().0).map(UniformMyF32) //! } -//! fn new_inclusive(low: B1, high: B2) -> Self +//! fn new_inclusive(low: B1, high: B2) -> Result //! where B1: SampleBorrow + Sized, //! B2: SampleBorrow + Sized //! { -//! UniformMyF32(UniformFloat::::new_inclusive( -//! low.borrow().0, -//! high.borrow().0, -//! )) +//! UniformFloat::::new_inclusive(low.borrow().0, high.borrow().0).map(UniformMyF32) //! } //! fn sample(&self, rng: &mut R) -> Self::X { //! MyF32(self.0.sample(rng)) @@ -95,7 +92,7 @@ //! } //! //! let (low, high) = (MyF32(17.0f32), MyF32(22.0f32)); -//! let uniform = Uniform::new(low, high); +//! let uniform = Uniform::new(low, high).unwrap(); //! let x = uniform.sample(&mut thread_rng()); //! ``` //! @@ -106,8 +103,10 @@ //! [`UniformDuration`]: crate::distributions::uniform::UniformDuration //! [`SampleBorrow::borrow`]: crate::distributions::uniform::SampleBorrow::borrow +use core::fmt; use core::time::Duration; use core::ops::{Range, RangeInclusive}; +use std::convert::TryFrom; use crate::distributions::float::IntoFloat; use crate::distributions::utils::{BoolAsSIMD, FloatAsSIMD, FloatSIMDUtils, WideningMultiply}; @@ -120,6 +119,29 @@ use crate::distributions::utils::Float; #[cfg(feature = "simd_support")] use packed_simd::*; +/// Error type returned from `Uniform::new{,_inclusive}`. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Error { + /// `low > high`, or equal in case of exclusive range. + EmptyRange, + /// Input or range `high - low` is non-finite. Not relevant to integer types. In release mode, + /// only the range is checked. + NonFinite, +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Error::EmptyRange => "low > high (or equal if exclusive) in uniform distribution", + Error::NonFinite => "Non-finite range in uniform distribution", + }) + } +} + +#[cfg(feature = "std")] +#[cfg_attr(doc_cfg, doc(cfg(feature = "std")))] +impl std::error::Error for Error {} + #[cfg(feature = "serde1")] use serde::{Serialize, Deserialize}; @@ -132,10 +154,10 @@ use serde::{Serialize, Deserialize}; /// /// When sampling from a constant range, many calculations can happen at /// compile-time and all methods should be fast; for floating-point ranges and -/// the full range of integer types this should have comparable performance to +/// the full range of integer types, this should have comparable performance to /// the `Standard` distribution. /// -/// Steps are taken to avoid bias which might be present in naive +/// Steps are taken to avoid bias, which might be present in naive /// implementations; for example `rng.gen::() % 170` samples from the range /// `[0, 169]` but is twice as likely to select numbers less than 85 than other /// values. Further, the implementations here give more weight to the high-bits @@ -149,9 +171,10 @@ use serde::{Serialize, Deserialize}; /// # Example /// /// ``` +/// use std::convert::TryFrom; /// use rand::distributions::{Distribution, Uniform}; /// -/// let between = Uniform::from(10..10000); +/// let between = Uniform::try_from(10..10000).unwrap(); /// let mut rng = rand::thread_rng(); /// let mut sum = 0; /// for _ in 0..1000 { @@ -179,24 +202,24 @@ use serde::{Serialize, Deserialize}; pub struct Uniform(X::Sampler); impl Uniform { - /// Create a new `Uniform` instance which samples uniformly from the half - /// open range `[low, high)` (excluding `high`). Panics if `low >= high`. - pub fn new(low: B1, high: B2) -> Uniform + /// Create a new `Uniform` instance, which samples uniformly from the half + /// open range `[low, high)` (excluding `high`). Fails if `low >= high`. + pub fn new(low: B1, high: B2) -> Result, Error> where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { - Uniform(X::Sampler::new(low, high)) + X::Sampler::new(low, high).map(Uniform) } - /// Create a new `Uniform` instance which samples uniformly from the closed - /// range `[low, high]` (inclusive). Panics if `low > high`. - pub fn new_inclusive(low: B1, high: B2) -> Uniform + /// Create a new `Uniform` instance, which samples uniformly from the closed + /// range `[low, high]` (inclusive). Fails if `low > high`. + pub fn new_inclusive(low: B1, high: B2) -> Result, Error> where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { - Uniform(X::Sampler::new_inclusive(low, high)) + X::Sampler::new_inclusive(low, high).map(Uniform) } } @@ -232,22 +255,18 @@ pub trait UniformSampler: Sized { /// The type sampled by this implementation. type X; - /// Construct self, with inclusive lower bound and exclusive upper bound - /// `[low, high)`. + /// Construct self, with inclusive lower bound and exclusive upper bound `[low, high)`. /// - /// Usually users should not call this directly but instead use - /// `Uniform::new`, which asserts that `low < high` before calling this. - fn new(low: B1, high: B2) -> Self + /// Usually users should not call this directly but instead use `Uniform::new`. + fn new(low: B1, high: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized; /// Construct self, with inclusive bounds `[low, high]`. /// - /// Usually users should not call this directly but instead use - /// `Uniform::new_inclusive`, which asserts that `low <= high` before - /// calling this. - fn new_inclusive(low: B1, high: B2) -> Self + /// Usually users should not call this directly but instead use `Uniform::new_inclusive`. + fn new_inclusive(low: B1, high: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized; @@ -279,7 +298,7 @@ pub trait UniformSampler: Sized { B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { - let uniform: Self = UniformSampler::new(low, high); + let uniform: Self = UniformSampler::new(low, high).unwrap(); uniform.sample(rng) } @@ -296,19 +315,23 @@ pub trait UniformSampler: Sized { where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized { - let uniform: Self = UniformSampler::new_inclusive(low, high); + let uniform: Self = UniformSampler::new_inclusive(low, high).unwrap(); uniform.sample(rng) } } -impl From> for Uniform { - fn from(r: ::core::ops::Range) -> Uniform { +impl TryFrom> for Uniform { + type Error = Error; + + fn try_from(r: ::core::ops::Range) -> Result, Error> { Uniform::new(r.start, r.end) } } -impl From> for Uniform { - fn from(r: ::core::ops::RangeInclusive) -> Uniform { +impl TryFrom> for Uniform { + type Error = Error; + + fn try_from(r: ::core::ops::RangeInclusive) -> Result, Error> { Uniform::new_inclusive(r.start(), r.end()) } } @@ -442,30 +465,31 @@ macro_rules! uniform_int_impl { #[inline] // if the range is constant, this helps LLVM to do the // calculations at compile-time. - fn new(low_b: B1, high_b: B2) -> Self + fn new(low_b: B1, high_b: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { let low = *low_b.borrow(); let high = *high_b.borrow(); - assert!(low < high, "Uniform::new called with `low >= high`"); + if !(low < high) { + return Err(Error::EmptyRange); + } UniformSampler::new_inclusive(low, high - 1) } #[inline] // if the range is constant, this helps LLVM to do the // calculations at compile-time. - fn new_inclusive(low_b: B1, high_b: B2) -> Self + fn new_inclusive(low_b: B1, high_b: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { let low = *low_b.borrow(); let high = *high_b.borrow(); - assert!( - low <= high, - "Uniform::new_inclusive called with `low > high`" - ); + if !(low <= high) { + return Err(Error::EmptyRange); + } let unsigned_max = ::core::$u_large::MAX; let range = high.wrapping_sub(low).wrapping_add(1) as $unsigned; @@ -476,12 +500,12 @@ macro_rules! uniform_int_impl { 0 }; - UniformInt { + Ok(UniformInt { low, // These are really $unsigned values, but store as $ty: range: range as $ty, z: ints_to_reject as $unsigned as $ty, - } + }) } #[inline] @@ -589,26 +613,29 @@ macro_rules! uniform_simd_int_impl { #[inline] // if the range is constant, this helps LLVM to do the // calculations at compile-time. - fn new(low_b: B1, high_b: B2) -> Self + fn new(low_b: B1, high_b: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized { let low = *low_b.borrow(); let high = *high_b.borrow(); - assert!(low.lt(high).all(), "Uniform::new called with `low >= high`"); + if !(low.lt(high).all()) { + return Err(Error::EmptyRange); + } UniformSampler::new_inclusive(low, high - 1) } #[inline] // if the range is constant, this helps LLVM to do the // calculations at compile-time. - fn new_inclusive(low_b: B1, high_b: B2) -> Self + fn new_inclusive(low_b: B1, high_b: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized { let low = *low_b.borrow(); let high = *high_b.borrow(); - assert!(low.le(high).all(), - "Uniform::new_inclusive called with `low > high`"); + if !(low.le(high).all()) { + return Err(Error::EmptyRange); + } let unsigned_max = ::core::$u_scalar::MAX; // NOTE: these may need to be replaced with explicitly @@ -625,12 +652,12 @@ macro_rules! uniform_simd_int_impl { // zero which means only one sample is needed. let zone = unsigned_max - ints_to_reject; - UniformInt { + Ok(UniformInt { low, // These are really $unsigned values, but store as $ty: range: range.cast(), z: zone.cast(), - } + }) } fn sample(&self, rng: &mut R) -> Self::X { @@ -751,7 +778,7 @@ impl UniformSampler for UniformChar { #[inline] // if the range is constant, this helps LLVM to do the // calculations at compile-time. - fn new(low_b: B1, high_b: B2) -> Self + fn new(low_b: B1, high_b: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, @@ -759,12 +786,12 @@ impl UniformSampler for UniformChar { let low = char_to_comp_u32(*low_b.borrow()); let high = char_to_comp_u32(*high_b.borrow()); let sampler = UniformInt::::new(low, high); - UniformChar { sampler } + sampler.map(|sampler| UniformChar { sampler }) } #[inline] // if the range is constant, this helps LLVM to do the // calculations at compile-time. - fn new_inclusive(low_b: B1, high_b: B2) -> Self + fn new_inclusive(low_b: B1, high_b: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, @@ -772,7 +799,7 @@ impl UniformSampler for UniformChar { let low = char_to_comp_u32(*low_b.borrow()); let high = char_to_comp_u32(*high_b.borrow()); let sampler = UniformInt::::new_inclusive(low, high); - UniformChar { sampler } + sampler.map(|sampler| UniformChar { sampler }) } fn sample(&self, rng: &mut R) -> Self::X { @@ -822,28 +849,29 @@ macro_rules! uniform_float_impl { impl UniformSampler for UniformFloat<$ty> { type X = $ty; - fn new(low_b: B1, high_b: B2) -> Self + fn new(low_b: B1, high_b: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { let low = *low_b.borrow(); let high = *high_b.borrow(); - debug_assert!( - low.all_finite(), - "Uniform::new called with `low` non-finite." - ); - debug_assert!( - high.all_finite(), - "Uniform::new called with `high` non-finite." - ); - assert!(low.all_lt(high), "Uniform::new called with `low >= high`"); + /* + if !(low.all_finite()) || !(high.all_finite()) { + return Err(Error::NonFinite); + } + */ + if !(low.all_lt(high)) { + return Err(Error::EmptyRange); + } let max_rand = <$ty>::splat( (::core::$u_scalar::MAX >> $bits_to_discard).into_float_with_exponent(0) - 1.0, ); let mut scale = high - low; - assert!(scale.all_finite(), "Uniform::new: range overflow"); + if !(scale.all_finite()) { + return Err(Error::NonFinite); + } loop { let mask = (scale * max_rand + low).ge_mask(high); @@ -855,34 +883,32 @@ macro_rules! uniform_float_impl { debug_assert!(<$ty>::splat(0.0).all_le(scale)); - UniformFloat { low, scale } + Ok(UniformFloat { low, scale }) } - fn new_inclusive(low_b: B1, high_b: B2) -> Self + fn new_inclusive(low_b: B1, high_b: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { let low = *low_b.borrow(); let high = *high_b.borrow(); - debug_assert!( - low.all_finite(), - "Uniform::new_inclusive called with `low` non-finite." - ); - debug_assert!( - high.all_finite(), - "Uniform::new_inclusive called with `high` non-finite." - ); - assert!( - low.all_le(high), - "Uniform::new_inclusive called with `low > high`" - ); + /* + if !(low.all_finite()) || !(high.all_finite()) { + return Err(Error::NonFinite); + } + */ + if !low.all_le(high) { + return Err(Error::EmptyRange); + } let max_rand = <$ty>::splat( (::core::$u_scalar::MAX >> $bits_to_discard).into_float_with_exponent(0) - 1.0, ); let mut scale = (high - low) / max_rand; - assert!(scale.all_finite(), "Uniform::new_inclusive: range overflow"); + if !scale.all_finite() { + return Err(Error::NonFinite); + } loop { let mask = (scale * max_rand + low).gt_mask(high); @@ -894,15 +920,15 @@ macro_rules! uniform_float_impl { debug_assert!(<$ty>::splat(0.0).all_le(scale)); - UniformFloat { low, scale } + Ok(UniformFloat { low, scale }) } fn sample(&self, rng: &mut R) -> Self::X { // Generate a value in the range [1, 2) let value1_2 = (rng.gen::<$uty>() >> $bits_to_discard).into_float_with_exponent(0); - // Get a value in the range [0, 1) in order to avoid - // overflowing into infinity when multiplying with scale + // Get a value in the range [0, 1) to avoid overflowing into infinity when + // multiplying with scale. let value0_1 = value1_2 - 1.0; // We don't use `f64::mul_add`, because it is not available with @@ -941,8 +967,8 @@ macro_rules! uniform_float_impl { let value1_2 = (rng.gen::<$uty>() >> $bits_to_discard).into_float_with_exponent(0); - // Get a value in the range [0, 1) in order to avoid - // overflowing into infinity when multiplying with scale + // Get a value in the range [0, 1) to avoid overflowing into infinity when + // multiplying with scale. let value0_1 = value1_2 - 1.0; // Doing multiply before addition allows some architectures @@ -979,7 +1005,7 @@ macro_rules! uniform_float_impl { // `high` are non-finite we'll end up here and can do the // appropriate checks. // - // Likewise `high - low` overflowing to infinity is also + // Likewise, `high - low` overflowing to infinity is also // rare, so handle it here after the common case. let mask = !scale.finite_mask(); if mask.any() { @@ -1051,29 +1077,30 @@ impl UniformSampler for UniformDuration { type X = Duration; #[inline] - fn new(low_b: B1, high_b: B2) -> Self + fn new(low_b: B1, high_b: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { let low = *low_b.borrow(); let high = *high_b.borrow(); - assert!(low < high, "Uniform::new called with `low >= high`"); + if !(low < high) { + return Err(Error::EmptyRange); + } UniformDuration::new_inclusive(low, high - Duration::new(0, 1)) } #[inline] - fn new_inclusive(low_b: B1, high_b: B2) -> Self + fn new_inclusive(low_b: B1, high_b: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { let low = *low_b.borrow(); let high = *high_b.borrow(); - assert!( - low <= high, - "Uniform::new_inclusive called with `low > high`" - ); + if !(low <= high) { + return Err(Error::EmptyRange); + } let low_s = low.as_secs(); let low_n = low.subsec_nanos(); @@ -1088,7 +1115,7 @@ impl UniformSampler for UniformDuration { let mode = if low_s == high_s { UniformDurationMode::Small { secs: low_s, - nanos: Uniform::new_inclusive(low_n, high_n), + nanos: Uniform::new_inclusive(low_n, high_n)?, } } else { let max = high_s @@ -1098,7 +1125,7 @@ impl UniformSampler for UniformDuration { if let Some(higher_bound) = max { let lower_bound = low_s * 1_000_000_000 + u64::from(low_n); UniformDurationMode::Medium { - nanos: Uniform::new_inclusive(lower_bound, higher_bound), + nanos: Uniform::new_inclusive(lower_bound, higher_bound)?, } } else { // An offset is applied to simplify generation of nanoseconds @@ -1106,14 +1133,14 @@ impl UniformSampler for UniformDuration { UniformDurationMode::Large { max_secs: high_s, max_nanos, - secs: Uniform::new_inclusive(low_s, high_s), + secs: Uniform::new_inclusive(low_s, high_s)?, } } }; - UniformDuration { + Ok(UniformDuration { mode, offset: low_n, - } + }) } #[inline] @@ -1133,7 +1160,7 @@ impl UniformSampler for UniformDuration { secs, } => { // constant folding means this is at least as fast as `Rng::sample(Range)` - let nano_range = Uniform::new(0, 1_000_000_000); + let nano_range = Uniform::new(0, 1_000_000_000).unwrap(); loop { let s = secs.sample(rng); let n = nano_range.sample(rng); @@ -1155,7 +1182,7 @@ mod tests { #[test] #[cfg(feature = "serde1")] fn test_serialization_uniform_duration() { - let distr = UniformDuration::new(Duration::from_secs(10), Duration::from_secs(60)); + let distr = UniformDuration::new(Duration::from_secs(10), Duration::from_secs(60)).unwrap(); let de_distr: UniformDuration = bincode::deserialize(&bincode::serialize(&distr).unwrap()).unwrap(); assert_eq!( distr.offset, de_distr.offset @@ -1188,39 +1215,37 @@ mod tests { #[test] #[cfg(feature = "serde1")] fn test_uniform_serialization() { - let unit_box: Uniform = Uniform::new(-1, 1); + let unit_box: Uniform = Uniform::new(-1, 1).unwrap(); let de_unit_box: Uniform = bincode::deserialize(&bincode::serialize(&unit_box).unwrap()).unwrap(); assert_eq!(unit_box.0.low, de_unit_box.0.low); assert_eq!(unit_box.0.range, de_unit_box.0.range); assert_eq!(unit_box.0.z, de_unit_box.0.z); - let unit_box: Uniform = Uniform::new(-1., 1.); + let unit_box: Uniform = Uniform::new(-1., 1.).unwrap(); let de_unit_box: Uniform = bincode::deserialize(&bincode::serialize(&unit_box).unwrap()).unwrap(); assert_eq!(unit_box.0.low, de_unit_box.0.low); assert_eq!(unit_box.0.scale, de_unit_box.0.scale); } - #[should_panic] #[test] fn test_uniform_bad_limits_equal_int() { - Uniform::new(10, 10); + assert_eq!(Uniform::new(10, 10), Err(Error::EmptyRange)); } #[test] fn test_uniform_good_limits_equal_int() { let mut rng = crate::test::rng(804); - let dist = Uniform::new_inclusive(10, 10); + let dist = Uniform::new_inclusive(10, 10).unwrap(); for _ in 0..20 { assert_eq!(rng.sample(dist), 10); } } - #[should_panic] #[test] fn test_uniform_bad_limits_flipped_int() { - Uniform::new(10, 5); + assert_eq!(Uniform::new(10, 5), Err(Error::EmptyRange)); } #[test] @@ -1234,25 +1259,25 @@ mod tests { macro_rules! t { ($ty:ident, $v:expr, $le:expr, $lt:expr) => {{ for &(low, high) in $v.iter() { - let my_uniform = Uniform::new(low, high); + let my_uniform = Uniform::new(low, high).unwrap(); for _ in 0..1000 { let v: $ty = rng.sample(my_uniform); assert!($le(low, v) && $lt(v, high)); } - let my_uniform = Uniform::new_inclusive(low, high); + let my_uniform = Uniform::new_inclusive(low, high).unwrap(); for _ in 0..1000 { let v: $ty = rng.sample(my_uniform); assert!($le(low, v) && $le(v, high)); } - let my_uniform = Uniform::new(&low, high); + let my_uniform = Uniform::new(&low, high).unwrap(); for _ in 0..1000 { let v: $ty = rng.sample(my_uniform); assert!($le(low, v) && $lt(v, high)); } - let my_uniform = Uniform::new_inclusive(&low, &high); + let my_uniform = Uniform::new_inclusive(&low, &high).unwrap(); for _ in 0..1000 { let v: $ty = rng.sample(my_uniform); assert!($le(low, v) && $le(v, high)); @@ -1323,7 +1348,7 @@ mod tests { let d = Uniform::new( core::char::from_u32(0xD7F0).unwrap(), core::char::from_u32(0xE010).unwrap(), - ); + ).unwrap(); for _ in 0..100 { let c = d.sample(&mut rng); assert!((c as u32) < 0xD800 || (c as u32) > 0xDFFF); @@ -1354,8 +1379,8 @@ mod tests { for lane in 0..<$ty>::lanes() { let low = <$ty>::splat(0.0 as $f_scalar).replace(lane, low_scalar); let high = <$ty>::splat(1.0 as $f_scalar).replace(lane, high_scalar); - let my_uniform = Uniform::new(low, high); - let my_incl_uniform = Uniform::new_inclusive(low, high); + let my_uniform = Uniform::new(low, high).unwrap(); + let my_incl_uniform = Uniform::new_inclusive(low, high).unwrap(); for _ in 0..100 { let v = rng.sample(my_uniform).extract(lane); assert!(low_scalar <= v && v < high_scalar); @@ -1367,7 +1392,7 @@ mod tests { } assert_eq!( - rng.sample(Uniform::new_inclusive(low, low)).extract(lane), + rng.sample(Uniform::new_inclusive(low, low).unwrap()).extract(lane), low_scalar ); @@ -1400,14 +1425,14 @@ mod tests { rng.sample(Uniform::new_inclusive( ::core::$f_scalar::MAX, ::core::$f_scalar::MAX - )), + ).unwrap()), ::core::$f_scalar::MAX ); assert_eq!( rng.sample(Uniform::new_inclusive( -::core::$f_scalar::MAX, -::core::$f_scalar::MAX - )), + ).unwrap()), -::core::$f_scalar::MAX ); }}; @@ -1428,9 +1453,8 @@ mod tests { } #[test] - #[should_panic] fn test_float_overflow() { - let _ = Uniform::from(::core::f64::MIN..::core::f64::MAX); + assert_eq!(Uniform::try_from(::core::f64::MIN..::core::f64::MAX), Err(Error::NonFinite)); } #[test] @@ -1478,10 +1502,10 @@ mod tests { let low = <$ty>::splat(0.0 as $f_scalar).replace(lane, low_scalar); let high = <$ty>::splat(1.0 as $f_scalar).replace(lane, high_scalar); assert!(catch_unwind(|| range(low, high)).is_err()); - assert!(catch_unwind(|| Uniform::new(low, high)).is_err()); - assert!(catch_unwind(|| Uniform::new_inclusive(low, high)).is_err()); + assert!(Uniform::new(low, high).is_err()); + assert!(Uniform::new_inclusive(low, high).is_err()); assert!(catch_unwind(|| range(low, low)).is_err()); - assert!(catch_unwind(|| Uniform::new(low, low)).is_err()); + assert!(Uniform::new(low, low).is_err()); } } }}; @@ -1516,7 +1540,7 @@ mod tests { ), ]; for &(low, high) in v.iter() { - let my_uniform = Uniform::new(low, high); + let my_uniform = Uniform::new(low, high).unwrap(); for _ in 0..1000 { let v = rng.sample(my_uniform); assert!(low <= v && v < high); @@ -1538,15 +1562,15 @@ mod tests { impl UniformSampler for UniformMyF32 { type X = MyF32; - fn new(low: B1, high: B2) -> Self + fn new(low: B1, high: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { - UniformMyF32(UniformFloat::::new(low.borrow().x, high.borrow().x)) + UniformFloat::::new(low.borrow().x, high.borrow().x).map(UniformMyF32) } - fn new_inclusive(low: B1, high: B2) -> Self + fn new_inclusive(low: B1, high: B2) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, @@ -1565,7 +1589,7 @@ mod tests { } let (low, high) = (MyF32 { x: 17.0f32 }, MyF32 { x: 22.0f32 }); - let uniform = Uniform::new(low, high); + let uniform = Uniform::new(low, high).unwrap(); let mut rng = crate::test::rng(804); for _ in 0..100 { let x: MyF32 = rng.sample(uniform); @@ -1575,20 +1599,20 @@ mod tests { #[test] fn test_uniform_from_std_range() { - let r = Uniform::from(2u32..7); + let r = Uniform::try_from(2u32..7).unwrap(); assert_eq!(r.0.low, 2); assert_eq!(r.0.range, 5); - let r = Uniform::from(2.0f64..7.0); + let r = Uniform::try_from(2.0f64..7.0).unwrap(); assert_eq!(r.0.low, 2.0); assert_eq!(r.0.scale, 5.0); } #[test] fn test_uniform_from_std_range_inclusive() { - let r = Uniform::from(2u32..=6); + let r = Uniform::try_from(2u32..=6).unwrap(); assert_eq!(r.0.low, 2); assert_eq!(r.0.range, 5); - let r = Uniform::from(2.0f64..=7.0); + let r = Uniform::try_from(2.0f64..=7.0).unwrap(); assert_eq!(r.0.low, 2.0); assert!(r.0.scale > 5.0); assert!(r.0.scale < 5.0 + 1e-14); @@ -1607,7 +1631,7 @@ mod tests { } assert_eq!(&buf, expected_single); - let distr = Uniform::new(lb, ub); + let distr = Uniform::new(lb, ub).unwrap(); for x in &mut buf { *x = rng.sample(&distr); } @@ -1650,9 +1674,9 @@ mod tests { #[test] fn uniform_distributions_can_be_compared() { - assert_eq!(Uniform::new(1.0, 2.0), Uniform::new(1.0, 2.0)); + assert_eq!(Uniform::new(1.0, 2.0).unwrap(), Uniform::new(1.0, 2.0).unwrap()); // To cover UniformInt - assert_eq!(Uniform::new(1 as u32, 2 as u32), Uniform::new(1 as u32, 2 as u32)); + assert_eq!(Uniform::new(1 as u32, 2 as u32).unwrap(), Uniform::new(1 as u32, 2 as u32).unwrap()); } } diff --git a/src/distributions/weighted_index.rs b/src/distributions/weighted_index.rs index 8252b172f7f..a34604cb2df 100644 --- a/src/distributions/weighted_index.rs +++ b/src/distributions/weighted_index.rs @@ -121,7 +121,7 @@ impl WeightedIndex { if total_weight == zero { return Err(WeightedError::AllWeightsZero); } - let distr = X::Sampler::new(zero, total_weight.clone()); + let distr = X::Sampler::new(zero, total_weight.clone()).unwrap(); Ok(WeightedIndex { cumulative_weights: weights, @@ -214,7 +214,7 @@ impl WeightedIndex { } self.total_weight = total_weight; - self.weight_distribution = X::Sampler::new(zero, self.total_weight.clone()); + self.weight_distribution = X::Sampler::new(zero, self.total_weight.clone()).unwrap(); Ok(()) } diff --git a/src/rng.rs b/src/rng.rs index 79a9fbff46e..743014f2c72 100644 --- a/src/rng.rs +++ b/src/rng.rs @@ -144,10 +144,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(Uniform::new(10u32, 15).unwrap()); /// // Type annotation requires two types, the type and distribution; the /// // distribution can be inferred. - /// let y = rng.sample::(Uniform::new(10, 15)); + /// let y = rng.sample::(Uniform::new(10, 15).unwrap()); /// ``` fn sample>(&mut self, distr: D) -> T { distr.sample(self) @@ -183,7 +183,7 @@ pub trait Rng: RngCore { /// .collect::>()); /// /// // Dice-rolling: - /// let die_range = Uniform::new_inclusive(1, 6); + /// let die_range = Uniform::new_inclusive(1, 6).unwrap(); /// let mut roll_die = (&mut rng).sample_iter(die_range); /// while roll_die.next().unwrap() != 6 { /// println!("Not a 6; rolling again!"); diff --git a/src/seq/index.rs b/src/seq/index.rs index b38e4649d1f..05aed07fbab 100644 --- a/src/seq/index.rs +++ b/src/seq/index.rs @@ -528,7 +528,7 @@ where let mut cache = HashSet::with_capacity(amount.as_usize()); #[cfg(not(feature = "std"))] let mut cache = BTreeSet::new(); - let distr = Uniform::new(X::zero(), length); + let distr = Uniform::new(X::zero(), length).unwrap(); let mut indices = Vec::with_capacity(amount.as_usize()); for _ in 0..amount.as_usize() { let mut pos = distr.sample(rng); From 9536f698b34d530a4fde990eee3a7aed1b0cce87 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Thu, 24 Nov 2022 19:30:09 +0100 Subject: [PATCH 3/7] Address review feedback --- src/distributions/uniform.rs | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/distributions/uniform.rs b/src/distributions/uniform.rs index 4be7a5c5e67..1297322be33 100644 --- a/src/distributions/uniform.rs +++ b/src/distributions/uniform.rs @@ -119,13 +119,12 @@ use crate::distributions::utils::Float; #[cfg(feature = "simd_support")] use packed_simd::*; -/// Error type returned from `Uniform::new{,_inclusive}`. +/// Error type returned from [`Uniform::new`] and `new_inclusive`. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Error { /// `low > high`, or equal in case of exclusive range. EmptyRange, - /// Input or range `high - low` is non-finite. Not relevant to integer types. In release mode, - /// only the range is checked. + /// Input or range `high - low` is non-finite. Not relevant to integer types. NonFinite, } @@ -203,7 +202,10 @@ pub struct Uniform(X::Sampler); impl Uniform { /// Create a new `Uniform` instance, which samples uniformly from the half - /// open range `[low, high)` (excluding `high`). Fails if `low >= high`. + /// open range `[low, high)` (excluding `high`). + /// + /// Fails if `low >= high`, or if `low`, `high` or the range `high - low` is + /// non-finite. In release mode, only the range is checked. pub fn new(low: B1, high: B2) -> Result, Error> where B1: SampleBorrow + Sized, @@ -213,7 +215,10 @@ impl Uniform { } /// Create a new `Uniform` instance, which samples uniformly from the closed - /// range `[low, high]` (inclusive). Fails if `low > high`. + /// range `[low, high]` (inclusive). + /// + /// Fails if `low > high`, or if `low`, `high` or the range `high - low` is + /// non-finite. In release mode, only the range is checked. pub fn new_inclusive(low: B1, high: B2) -> Result, Error> where B1: SampleBorrow + Sized, @@ -257,7 +262,8 @@ pub trait UniformSampler: Sized { /// Construct self, with inclusive lower bound and exclusive upper bound `[low, high)`. /// - /// Usually users should not call this directly but instead use `Uniform::new`. + /// Usually users should not call this directly but prefer to use + /// [`Uniform::new`]. fn new(low: B1, high: B2) -> Result where B1: SampleBorrow + Sized, @@ -265,7 +271,8 @@ pub trait UniformSampler: Sized { /// Construct self, with inclusive bounds `[low, high]`. /// - /// Usually users should not call this directly but instead use `Uniform::new_inclusive`. + /// Usually users should not call this directly but prefer to use + /// [`Uniform::new_inclusive`]. fn new_inclusive(low: B1, high: B2) -> Result where B1: SampleBorrow + Sized, @@ -856,11 +863,10 @@ macro_rules! uniform_float_impl { { let low = *low_b.borrow(); let high = *high_b.borrow(); - /* + #[cfg(debug_assertions)] if !(low.all_finite()) || !(high.all_finite()) { return Err(Error::NonFinite); } - */ if !(low.all_lt(high)) { return Err(Error::EmptyRange); } @@ -893,11 +899,10 @@ macro_rules! uniform_float_impl { { let low = *low_b.borrow(); let high = *high_b.borrow(); - /* + #[cfg(debug_assertions)] if !(low.all_finite()) || !(high.all_finite()) { return Err(Error::NonFinite); } - */ if !low.all_le(high) { return Err(Error::EmptyRange); } From 0754b72ad1d94e20e631241651716aeb78479fa0 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Tue, 6 Dec 2022 20:56:19 +0100 Subject: [PATCH 4/7] Make `sample_single` return a `Result` --- src/distributions/uniform.rs | 84 ++++++++++++++++++------------------ src/rng.rs | 2 +- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/distributions/uniform.rs b/src/distributions/uniform.rs index 1297322be33..2a36a3e3487 100644 --- a/src/distributions/uniform.rs +++ b/src/distributions/uniform.rs @@ -297,16 +297,16 @@ pub trait UniformSampler: Sized { /// # #[allow(unused)] /// fn sample_from_range(lb: T, ub: T) -> T { /// let mut rng = thread_rng(); - /// ::Sampler::sample_single(lb, ub, &mut rng) + /// ::Sampler::sample_single(lb, ub, &mut rng).unwrap() /// } /// ``` - fn sample_single(low: B1, high: B2, rng: &mut R) -> Self::X + fn sample_single(low: B1, high: B2, rng: &mut R) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { - let uniform: Self = UniformSampler::new(low, high).unwrap(); - uniform.sample(rng) + let uniform: Self = UniformSampler::new(low, high)?; + Ok(uniform.sample(rng)) } /// Sample a single value uniformly from a range with inclusive lower bound @@ -318,12 +318,12 @@ pub trait UniformSampler: Sized { /// via this method. /// Results may not be identical. fn sample_single_inclusive(low: B1, high: B2, rng: &mut R) - -> Self::X + -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized { - let uniform: Self = UniformSampler::new_inclusive(low, high).unwrap(); - uniform.sample(rng) + let uniform: Self = UniformSampler::new_inclusive(low, high)?; + Ok(uniform.sample(rng)) } } @@ -378,7 +378,7 @@ where Borrowed: SampleUniform /// for `Rng::gen_range`. pub trait SampleRange { /// Generate a sample from the given range. - fn sample_single(self, rng: &mut R) -> T; + fn sample_single(self, rng: &mut R) -> Result; /// Check whether the range is empty. fn is_empty(&self) -> bool; @@ -386,7 +386,7 @@ pub trait SampleRange { impl SampleRange for Range { #[inline] - fn sample_single(self, rng: &mut R) -> T { + fn sample_single(self, rng: &mut R) -> Result { T::Sampler::sample_single(self.start, self.end, rng) } @@ -398,7 +398,7 @@ impl SampleRange for Range { impl SampleRange for RangeInclusive { #[inline] - fn sample_single(self, rng: &mut R) -> T { + fn sample_single(self, rng: &mut R) -> Result { T::Sampler::sample_single_inclusive(self.start(), self.end(), rng) } @@ -535,31 +535,35 @@ macro_rules! uniform_int_impl { } #[inline] - fn sample_single(low_b: B1, high_b: B2, rng: &mut R) -> Self::X + fn sample_single(low_b: B1, high_b: B2, rng: &mut R) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { let low = *low_b.borrow(); let high = *high_b.borrow(); - assert!(low < high, "UniformSampler::sample_single: low >= high"); + if !(low < high) { + return Err(Error::EmptyRange); + } Self::sample_single_inclusive(low, high - 1, rng) } #[inline] - fn sample_single_inclusive(low_b: B1, high_b: B2, rng: &mut R) -> Self::X + fn sample_single_inclusive(low_b: B1, high_b: B2, rng: &mut R) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { let low = *low_b.borrow(); let high = *high_b.borrow(); - assert!(low <= high, "UniformSampler::sample_single_inclusive: low > high"); + if !(low <= high) { + return Err(Error::EmptyRange); + } let range = high.wrapping_sub(low).wrapping_add(1) as $unsigned as $u_large; // If the above resulted in wrap-around to 0, the range is $ty::MIN..=$ty::MAX, // and any integer will do. if range == 0 { - return rng.gen(); + return Ok(rng.gen()); } let zone = if ::core::$unsigned::MAX <= ::core::u16::MAX as $unsigned { @@ -579,7 +583,7 @@ macro_rules! uniform_int_impl { let v: $u_large = rng.gen(); let (hi, lo) = v.wmul(range); if lo <= zone { - return low.wrapping_add(hi as $ty); + return Ok(low.wrapping_add(hi as $ty)); } } } @@ -945,27 +949,24 @@ macro_rules! uniform_float_impl { } #[inline] - fn sample_single(low_b: B1, high_b: B2, rng: &mut R) -> Self::X + fn sample_single(low_b: B1, high_b: B2, rng: &mut R) -> Result where B1: SampleBorrow + Sized, B2: SampleBorrow + Sized, { let low = *low_b.borrow(); let high = *high_b.borrow(); - debug_assert!( - low.all_finite(), - "UniformSampler::sample_single called with `low` non-finite." - ); - debug_assert!( - high.all_finite(), - "UniformSampler::sample_single called with `high` non-finite." - ); - assert!( - low.all_lt(high), - "UniformSampler::sample_single: low >= high" - ); + #[cfg(debug_assertions)] + if !low.all_finite() || !high.all_finite() { + return Err(Error::NonFinite); + } + if !low.all_lt(high) { + return Err(Error::EmptyRange); + } let mut scale = high - low; - assert!(scale.all_finite(), "UniformSampler::sample_single: range overflow"); + if !scale.all_finite() { + return Err(Error::NonFinite); + } loop { // Generate a value in the range [1, 2) @@ -982,7 +983,7 @@ macro_rules! uniform_float_impl { debug_assert!(low.all_le(res) || !scale.all_finite()); if res.all_lt(high) { - return res; + return Ok(res); } // This handles a number of edge cases. @@ -1014,10 +1015,9 @@ macro_rules! uniform_float_impl { // rare, so handle it here after the common case. let mask = !scale.finite_mask(); if mask.any() { - assert!( - low.all_finite() && high.all_finite(), - "Uniform::sample_single: low and high must be finite" - ); + if !(low.all_finite() && high.all_finite()) { + return Err(Error::NonFinite); + } scale = scale.decrease_masked(mask); } } @@ -1289,12 +1289,12 @@ mod tests { } for _ in 0..1000 { - let v = <$ty as SampleUniform>::Sampler::sample_single(low, high, &mut rng); + let v = <$ty as SampleUniform>::Sampler::sample_single(low, high, &mut rng).unwrap(); assert!($le(low, v) && $lt(v, high)); } for _ in 0..1000 { - let v = <$ty as SampleUniform>::Sampler::sample_single_inclusive(low, high, &mut rng); + let v = <$ty as SampleUniform>::Sampler::sample_single_inclusive(low, high, &mut rng).unwrap(); assert!($le(low, v) && $le(v, high)); } } @@ -1392,7 +1392,7 @@ mod tests { let v = rng.sample(my_incl_uniform).extract(lane); assert!(low_scalar <= v && v <= high_scalar); let v = <$ty as SampleUniform>::Sampler - ::sample_single(low, high, &mut rng).extract(lane); + ::sample_single(low, high, &mut rng).unwrap().extract(lane); assert!(low_scalar <= v && v < high_scalar); } @@ -1404,7 +1404,7 @@ mod tests { assert_eq!(zero_rng.sample(my_uniform).extract(lane), low_scalar); assert_eq!(zero_rng.sample(my_incl_uniform).extract(lane), low_scalar); assert_eq!(<$ty as SampleUniform>::Sampler - ::sample_single(low, high, &mut zero_rng) + ::sample_single(low, high, &mut zero_rng).unwrap() .extract(lane), low_scalar); assert!(max_rng.sample(my_uniform).extract(lane) < high_scalar); assert!(max_rng.sample(my_incl_uniform).extract(lane) <= high_scalar); @@ -1419,7 +1419,7 @@ mod tests { ); assert!( <$ty as SampleUniform>::Sampler - ::sample_single(low, high, &mut lowering_max_rng) + ::sample_single(low, high, &mut lowering_max_rng).unwrap() .extract(lane) < high_scalar ); } @@ -1480,7 +1480,7 @@ mod tests { use std::panic::catch_unwind; fn range(low: T, high: T) { let mut rng = crate::test::rng(253); - T::Sampler::sample_single(low, high, &mut rng); + T::Sampler::sample_single(low, high, &mut rng).unwrap(); } macro_rules! t { @@ -1632,7 +1632,7 @@ mod tests { let mut buf = [lb; 3]; for x in &mut buf { - *x = T::Sampler::sample_single(lb, ub, &mut rng); + *x = T::Sampler::sample_single(lb, ub, &mut rng).unwrap(); } assert_eq!(&buf, expected_single); diff --git a/src/rng.rs b/src/rng.rs index 743014f2c72..ee7152003cd 100644 --- a/src/rng.rs +++ b/src/rng.rs @@ -132,7 +132,7 @@ pub trait Rng: RngCore { R: SampleRange { assert!(!range.is_empty(), "cannot sample empty range"); - range.sample_single(self) + range.sample_single(self).unwrap() } /// Sample a new value, using the given distribution. From f20b74ef2cd4499516e68791a53868196c5da796 Mon Sep 17 00:00:00 2001 From: Vinzent Steinberg Date: Tue, 6 Dec 2022 21:01:16 +0100 Subject: [PATCH 5/7] Fix benchmarks --- benches/distributions.rs | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/benches/distributions.rs b/benches/distributions.rs index 76d5d258d9d..f637fe4ae47 100644 --- a/benches/distributions.rs +++ b/benches/distributions.rs @@ -131,36 +131,36 @@ macro_rules! distr { } // uniform -distr_int!(distr_uniform_i8, i8, Uniform::new(20i8, 100)); -distr_int!(distr_uniform_i16, i16, Uniform::new(-500i16, 2000)); -distr_int!(distr_uniform_i32, i32, Uniform::new(-200_000_000i32, 800_000_000)); -distr_int!(distr_uniform_i64, i64, Uniform::new(3i64, 123_456_789_123)); -distr_int!(distr_uniform_i128, i128, Uniform::new(-123_456_789_123i128, 123_456_789_123_456_789)); -distr_int!(distr_uniform_usize16, usize, Uniform::new(0usize, 0xb9d7)); -distr_int!(distr_uniform_usize32, usize, Uniform::new(0usize, 0x548c0f43)); +distr_int!(distr_uniform_i8, i8, Uniform::new(20i8, 100).unwrap()); +distr_int!(distr_uniform_i16, i16, Uniform::new(-500i16, 2000).unwrap()); +distr_int!(distr_uniform_i32, i32, Uniform::new(-200_000_000i32, 800_000_000).unwrap()); +distr_int!(distr_uniform_i64, i64, Uniform::new(3i64, 123_456_789_123).unwrap()); +distr_int!(distr_uniform_i128, i128, Uniform::new(-123_456_789_123i128, 123_456_789_123_456_789).unwrap()); +distr_int!(distr_uniform_usize16, usize, Uniform::new(0usize, 0xb9d7).unwrap()); +distr_int!(distr_uniform_usize32, usize, Uniform::new(0usize, 0x548c0f43).unwrap()); #[cfg(target_pointer_width = "64")] -distr_int!(distr_uniform_usize64, usize, Uniform::new(0usize, 0x3a42714f2bf927a8)); -distr_int!(distr_uniform_isize, isize, Uniform::new(-1060478432isize, 1858574057)); +distr_int!(distr_uniform_usize64, usize, Uniform::new(0usize, 0x3a42714f2bf927a8).unwrap()); +distr_int!(distr_uniform_isize, isize, Uniform::new(-1060478432isize, 1858574057).unwrap()); -distr_float!(distr_uniform_f32, f32, Uniform::new(2.26f32, 2.319)); -distr_float!(distr_uniform_f64, f64, Uniform::new(2.26f64, 2.319)); +distr_float!(distr_uniform_f32, f32, Uniform::new(2.26f32, 2.319).unwrap()); +distr_float!(distr_uniform_f64, f64, Uniform::new(2.26f64, 2.319).unwrap()); const LARGE_SEC: u64 = u64::max_value() / 1000; distr_duration!(distr_uniform_duration_largest, - Uniform::new_inclusive(Duration::new(0, 0), Duration::new(u64::max_value(), 999_999_999)) + Uniform::new_inclusive(Duration::new(0, 0), Duration::new(u64::max_value(), 999_999_999)).unwrap() ); distr_duration!(distr_uniform_duration_large, - Uniform::new(Duration::new(0, 0), Duration::new(LARGE_SEC, 1_000_000_000 / 2)) + Uniform::new(Duration::new(0, 0), Duration::new(LARGE_SEC, 1_000_000_000 / 2)).unwrap() ); distr_duration!(distr_uniform_duration_one, - Uniform::new(Duration::new(0, 0), Duration::new(1, 0)) + Uniform::new(Duration::new(0, 0), Duration::new(1, 0)).unwrap() ); distr_duration!(distr_uniform_duration_variety, - Uniform::new(Duration::new(10000, 423423), Duration::new(200000, 6969954)) + Uniform::new(Duration::new(10000, 423423), Duration::new(200000, 6969954)).unwrap() ); distr_duration!(distr_uniform_duration_edge, - Uniform::new_inclusive(Duration::new(LARGE_SEC, 999_999_999), Duration::new(LARGE_SEC + 1, 1)) + Uniform::new_inclusive(Duration::new(LARGE_SEC, 999_999_999), Duration::new(LARGE_SEC + 1, 1)).unwrap() ); // standard @@ -272,7 +272,7 @@ macro_rules! uniform_sample { let high = black_box($high); b.iter(|| { for _ in 0..10 { - let dist = UniformInt::<$type>::new(low, high); + let dist = UniformInt::<$type>::new(low, high).unwrap(); for _ in 0..$count { black_box(dist.sample(&mut rng)); } @@ -291,7 +291,7 @@ macro_rules! uniform_inclusive { let high = black_box($high); b.iter(|| { for _ in 0..10 { - let dist = UniformInt::<$type>::new_inclusive(low, high); + let dist = UniformInt::<$type>::new_inclusive(low, high).unwrap(); for _ in 0..$count { black_box(dist.sample(&mut rng)); } @@ -311,7 +311,7 @@ macro_rules! uniform_single { let high = black_box($high); b.iter(|| { for _ in 0..(10 * $count) { - black_box(UniformInt::<$type>::sample_single(low, high, &mut rng)); + black_box(UniformInt::<$type>::sample_single(low, high, &mut rng).unwrap()); } }); } From 6695abe4ba72ab30ecffb2fd82018a9ecd12db71 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Fri, 3 Feb 2023 12:04:16 +0000 Subject: [PATCH 6/7] Small fixes --- examples/rayon-monte-carlo.rs | 2 +- src/distributions/uniform.rs | 2 +- src/distributions/weighted_index.rs | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/examples/rayon-monte-carlo.rs b/examples/rayon-monte-carlo.rs index f0e7114a657..7e703c01d2d 100644 --- a/examples/rayon-monte-carlo.rs +++ b/examples/rayon-monte-carlo.rs @@ -49,7 +49,7 @@ static BATCH_SIZE: u64 = 10_000; static BATCHES: u64 = 1000; fn main() { - let range = Uniform::new(-1.0f64, 1.0); + let range = Uniform::new(-1.0f64, 1.0).unwrap(); let in_circle = (0..BATCHES) .into_par_iter() diff --git a/src/distributions/uniform.rs b/src/distributions/uniform.rs index 784b93a4a67..91f79d79818 100644 --- a/src/distributions/uniform.rs +++ b/src/distributions/uniform.rs @@ -106,7 +106,7 @@ use core::fmt; use core::time::Duration; use core::ops::{Range, RangeInclusive}; -use std::convert::TryFrom; +use core::convert::TryFrom; use crate::distributions::float::IntoFloat; use crate::distributions::utils::{BoolAsSIMD, FloatAsSIMD, FloatSIMDUtils, IntAsSIMD, WideningMultiply}; diff --git a/src/distributions/weighted_index.rs b/src/distributions/weighted_index.rs index ced241327dc..4c57edc5f60 100644 --- a/src/distributions/weighted_index.rs +++ b/src/distributions/weighted_index.rs @@ -230,7 +230,6 @@ impl Distribution for WeightedIndex where X: SampleUniform + PartialOrd { fn sample(&self, rng: &mut R) -> usize { - use ::core::cmp::Ordering; let chosen_weight = self.weight_distribution.sample(rng); // Find the first item which has a weight *higher* than the chosen weight. self.cumulative_weights.partition_point(|w| w <= &chosen_weight) From 0eb674ba8fa3c6e92e649eb03f7aa46bbb83d85f Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Mon, 6 Feb 2023 10:43:55 +0000 Subject: [PATCH 7/7] Update src/distributions/uniform.rs --- src/distributions/uniform.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/distributions/uniform.rs b/src/distributions/uniform.rs index 91f79d79818..b4856ff6131 100644 --- a/src/distributions/uniform.rs +++ b/src/distributions/uniform.rs @@ -172,7 +172,6 @@ use serde::{Serialize, Deserialize}; /// # Example /// /// ``` -/// use std::convert::TryFrom; /// use rand::distributions::{Distribution, Uniform}; /// /// let between = Uniform::try_from(10..10000).unwrap();