rust-random / rand

A Rust library for random number generation.
https://crates.io/crates/rand
Other
1.67k stars 431 forks source link

Uniform::new: range overflow #1380

Closed cksac closed 9 months ago

cksac commented 9 months ago

Is it expected?

use rand::distributions::{Distribution, Uniform};

fn main() {
    // ok
    let r = Uniform::new(0.0, f32::MAX);
    // range overflow
    let r = Uniform::new_inclusive(0.0, f32::MAX);
    // range overflow
    let r = Uniform::new(f32::MIN, f32::MAX);
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b1cd60bdecf24a68daca0a86c08a2fbe

TheIronBorn commented 9 months ago

changes are coming to this in #1229

cksac commented 9 months ago

looks like it is expected, when (high - low) larger than the type can hold. e.g.

fn main() {
    let scale = f32::MAX - f32::MIN;
    println!("{}", f32::MAX); // 340282350000000000000000000000000000000
    println!("{}", f32::MIN); // -340282350000000000000000000000000000000
    println!("{}", scale);  //inf
    println!("{}", scale.is_finite()); //false
}
cksac commented 9 months ago

let r = Uniform::new_inclusive(0.0, f32::MAX); shouldn't overflow?


use rand::distributions::{Distribution, Uniform};

fn main() {
    // ok
    let r = Uniform::new(0.0, f32::MAX);
    // range overflow expected? as MAX - 0 = MAX
    let r = Uniform::new_inclusive(0.0, f32::MAX);
    // range overflow should be expected, as MAX - MIN > MAX
    let r = Uniform::new(f32::MIN, f32::MAX);
}
dhardy commented 9 months ago

The current algorithm for new_inclusive for floats divides the range by max_rand which is approx. 0.9999999 to ensure both end points are representable. This results in an overflow when the range is already f32::MAX. It's a corner case, but one I don't see any need to support.