From 5c005600fef00fbf5fc044589e575f410a0ebb9b Mon Sep 17 00:00:00 2001 From: Benjamin Lieser Date: Thu, 26 Sep 2024 12:36:51 +0200 Subject: [PATCH 1/5] Poisson max_lambda and u64 sampling --- rand_distr/src/poisson.rs | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/rand_distr/src/poisson.rs b/rand_distr/src/poisson.rs index 26e7712b2c..2ee3624972 100644 --- a/rand_distr/src/poisson.rs +++ b/rand_distr/src/poisson.rs @@ -40,7 +40,7 @@ use rand::Rng; /// use rand_distr::{Poisson, Distribution}; /// /// let poi = Poisson::new(2.0).unwrap(); -/// let v = poi.sample(&mut rand::thread_rng()); +/// let v : f64 = poi.sample(&mut rand::thread_rng()); /// println!("{} is from a Poisson(2) distribution", v); /// ``` #[derive(Clone, Copy, Debug, PartialEq)] @@ -59,6 +59,8 @@ pub enum Error { ShapeTooSmall, /// `lambda = ∞` or `lambda = nan` NonFinite, + /// `lambda` is too large, we disallo + ShapeTooLarge } impl fmt::Display for Error { @@ -66,6 +68,7 @@ impl fmt::Display for Error { f.write_str(match self { Error::ShapeTooSmall => "lambda is not positive in Poisson distribution", Error::NonFinite => "lambda is infinite or nan in Poisson distribution", + Error::ShapeTooLarge => "lambda is too large in Poisson distribution, see Poisson::MAX_LAMBDA_POISSON", }) } } @@ -124,15 +127,8 @@ where { /// Construct a new `Poisson` with the given shape parameter /// `lambda`. - /// - /// # Known issues - /// - /// Although this method should return an [`Error`] on invalid parameters, - /// some (extreme) values of `lambda` are known to return a [`Poisson`] - /// object which hangs when [sampled](Distribution::sample). - /// Large (less extreme) values of `lambda` may result in successful - /// sampling but with reduced precision. - /// See [#1312](https://github.com/rust-random/rand/issues/1312). + /// + /// The maximum allowed lambda is [MAX_LAMBDA_POISSON](Self::MAX_LAMBDA_POISSON). pub fn new(lambda: F) -> Result, Error> { if !lambda.is_finite() { return Err(Error::NonFinite); @@ -145,11 +141,22 @@ where let method = if lambda < F::from(12.0).unwrap() { Method::Knuth(KnuthMethod::new(lambda)) } else { + if lambda > F::from(Self::MAX_LAMBDA_POISSON).unwrap() { + return Err(Error::ShapeTooLarge); + } Method::Rejection(RejectionMethod::new(lambda)) }; Ok(Poisson(method)) } + + /// Maximum value for `lambda` in the Poisson distribution. + /// This will make sure the samples will fit in a `u64`. + /// It also makes sure we do not run into numerical problems with very large `lambda`. + /// `1.844e19 + 1_000_000 * sqrt(1.844e19) < 2^64 - 1`, + /// so the probability of getting a value larger than `u64::MAX` is << 1e-1000. + pub const MAX_LAMBDA_POISSON: f64 = 1.844e19; + } impl Distribution for KnuthMethod @@ -232,6 +239,14 @@ where } } +impl Distribution for Poisson +{ + #[inline] + fn sample(&self, rng: &mut R) -> u64 { + as Distribution>::sample(self,rng) as u64 + } +} + #[cfg(test)] mod test { use super::*; From c3d96fa761969b77f8bcaaa9733a0053274c67d9 Mon Sep 17 00:00:00 2001 From: Benjamin Lieser Date: Thu, 26 Sep 2024 12:50:29 +0200 Subject: [PATCH 2/5] fmt --- rand_distr/src/poisson.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/rand_distr/src/poisson.rs b/rand_distr/src/poisson.rs index 2ee3624972..c99da761e3 100644 --- a/rand_distr/src/poisson.rs +++ b/rand_distr/src/poisson.rs @@ -60,7 +60,7 @@ pub enum Error { /// `lambda = ∞` or `lambda = nan` NonFinite, /// `lambda` is too large, we disallo - ShapeTooLarge + ShapeTooLarge, } impl fmt::Display for Error { @@ -68,7 +68,9 @@ impl fmt::Display for Error { f.write_str(match self { Error::ShapeTooSmall => "lambda is not positive in Poisson distribution", Error::NonFinite => "lambda is infinite or nan in Poisson distribution", - Error::ShapeTooLarge => "lambda is too large in Poisson distribution, see Poisson::MAX_LAMBDA_POISSON", + Error::ShapeTooLarge => { + "lambda is too large in Poisson distribution, see Poisson::MAX_LAMBDA_POISSON" + } }) } } @@ -127,7 +129,7 @@ where { /// Construct a new `Poisson` with the given shape parameter /// `lambda`. - /// + /// /// The maximum allowed lambda is [MAX_LAMBDA_POISSON](Self::MAX_LAMBDA_POISSON). pub fn new(lambda: F) -> Result, Error> { if !lambda.is_finite() { @@ -149,14 +151,13 @@ where Ok(Poisson(method)) } - + /// Maximum value for `lambda` in the Poisson distribution. /// This will make sure the samples will fit in a `u64`. /// It also makes sure we do not run into numerical problems with very large `lambda`. /// `1.844e19 + 1_000_000 * sqrt(1.844e19) < 2^64 - 1`, /// so the probability of getting a value larger than `u64::MAX` is << 1e-1000. pub const MAX_LAMBDA_POISSON: f64 = 1.844e19; - } impl Distribution for KnuthMethod @@ -239,11 +240,10 @@ where } } -impl Distribution for Poisson -{ +impl Distribution for Poisson { #[inline] fn sample(&self, rng: &mut R) -> u64 { - as Distribution>::sample(self,rng) as u64 + as Distribution>::sample(self, rng) as u64 } } From 977f69f9b81bec3a9acef2d5fd28e23acf32a6ca Mon Sep 17 00:00:00 2001 From: Benjamin Lieser Date: Sat, 28 Sep 2024 20:51:23 +0200 Subject: [PATCH 3/5] apply review --- rand_distr/src/poisson.rs | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/rand_distr/src/poisson.rs b/rand_distr/src/poisson.rs index c99da761e3..e65185a6a3 100644 --- a/rand_distr/src/poisson.rs +++ b/rand_distr/src/poisson.rs @@ -40,7 +40,7 @@ use rand::Rng; /// use rand_distr::{Poisson, Distribution}; /// /// let poi = Poisson::new(2.0).unwrap(); -/// let v : f64 = poi.sample(&mut rand::thread_rng()); +/// let v: f64 = poi.sample(&mut rand::thread_rng()); /// println!("{} is from a Poisson(2) distribution", v); /// ``` #[derive(Clone, Copy, Debug, PartialEq)] @@ -52,14 +52,12 @@ where /// Error type returned from [`Poisson::new`]. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -// Marked non_exhaustive to allow a new error code in the solution to #1312. -#[non_exhaustive] pub enum Error { /// `lambda <= 0` ShapeTooSmall, /// `lambda = ∞` or `lambda = nan` NonFinite, - /// `lambda` is too large, we disallo + /// `lambda` is too large, see [Poisson::MAX_LAMBDA] ShapeTooLarge, } @@ -69,7 +67,7 @@ impl fmt::Display for Error { Error::ShapeTooSmall => "lambda is not positive in Poisson distribution", Error::NonFinite => "lambda is infinite or nan in Poisson distribution", Error::ShapeTooLarge => { - "lambda is too large in Poisson distribution, see Poisson::MAX_LAMBDA_POISSON" + "lambda is too large in Poisson distribution, see Poisson::MAX_LAMBDA" } }) } @@ -130,7 +128,7 @@ where /// Construct a new `Poisson` with the given shape parameter /// `lambda`. /// - /// The maximum allowed lambda is [MAX_LAMBDA_POISSON](Self::MAX_LAMBDA_POISSON). + /// The maximum allowed lambda is [MAX_LAMBDA](Self::MAX_LAMBDA). pub fn new(lambda: F) -> Result, Error> { if !lambda.is_finite() { return Err(Error::NonFinite); @@ -143,7 +141,7 @@ where let method = if lambda < F::from(12.0).unwrap() { Method::Knuth(KnuthMethod::new(lambda)) } else { - if lambda > F::from(Self::MAX_LAMBDA_POISSON).unwrap() { + if lambda > F::from(Self::MAX_LAMBDA).unwrap() { return Err(Error::ShapeTooLarge); } Method::Rejection(RejectionMethod::new(lambda)) @@ -152,12 +150,16 @@ where Ok(Poisson(method)) } - /// Maximum value for `lambda` in the Poisson distribution. - /// This will make sure the samples will fit in a `u64`. - /// It also makes sure we do not run into numerical problems with very large `lambda`. - /// `1.844e19 + 1_000_000 * sqrt(1.844e19) < 2^64 - 1`, - /// so the probability of getting a value larger than `u64::MAX` is << 1e-1000. - pub const MAX_LAMBDA_POISSON: f64 = 1.844e19; + /// The maximum supported value of `lambda` + /// + /// This value was selected such that + /// `MAX_LAMBDA + 1e6 * sqrt(MAX_LAMBDA) < 2^64 - 1`, + /// thus ensuring that the probability of sampling a value larger than + /// `u64::MAX` is less than 1e-1000. + /// + /// Applying this limit also solves + /// [#1312](https://github.com/rust-random/rand/issues/1312). + pub const MAX_LAMBDA: f64 = 1.844e19; } impl Distribution for KnuthMethod @@ -243,6 +245,7 @@ where impl Distribution for Poisson { #[inline] fn sample(&self, rng: &mut R) -> u64 { + // `as` from float to int saturates as Distribution>::sample(self, rng) as u64 } } From 11d843384c363ac1d21243d5b86f1b2070646114 Mon Sep 17 00:00:00 2001 From: Benjamin Lieser Date: Sat, 28 Sep 2024 20:52:38 +0200 Subject: [PATCH 4/5] remove known issues doc --- rand_distr/src/poisson.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rand_distr/src/poisson.rs b/rand_distr/src/poisson.rs index e65185a6a3..759f39cde7 100644 --- a/rand_distr/src/poisson.rs +++ b/rand_distr/src/poisson.rs @@ -23,10 +23,6 @@ use rand::Rng; /// This distribution has density function: /// `f(k) = λ^k * exp(-λ) / k!` for `k >= 0`. /// -/// # Known issues -/// -/// See documentation of [`Poisson::new`]. -/// /// # Plot /// /// The following plot shows the Poisson distribution with various values of `λ`. From 48650a725ac33dcc00b003b697876dfee4e04b0d Mon Sep 17 00:00:00 2001 From: Benjamin Lieser Date: Sat, 28 Sep 2024 20:58:57 +0200 Subject: [PATCH 5/5] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d071e09391..15347e017d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ You may also find the [Upgrade Guide](https://rust-random.github.io/book/update. - Add `UniformUsize` and use to make `Uniform` for `usize` portable (#1487) - Remove support for generating `isize` and `usize` values with `Standard`, `Uniform` and `Fill` and usage as a `WeightedAliasIndex` weight (#1487) - Require `Clone` and `AsRef` bound for `SeedableRng::Seed`. (#1491) +- Implement `Distribution` for `Poisson` (#1498) +- Limit the maximal acceptable lambda for `Poisson` to solve (#1312) (#1498) ## [0.9.0-alpha.1] - 2024-03-18 - Add the `Slice::num_choices` method to the Slice distribution (#1402)