Skip to content
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

Uniform::new: range overflow #1380

Closed
cksac opened this issue Feb 8, 2024 · 4 comments
Closed

Uniform::new: range overflow #1380

cksac opened this issue Feb 8, 2024 · 4 comments

Comments

@cksac
Copy link

cksac commented Feb 8, 2024

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
Copy link
Collaborator

changes are coming to this in #1229

@cksac
Copy link
Author

cksac commented Feb 8, 2024

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 cksac closed this as completed Feb 8, 2024
@cksac
Copy link
Author

cksac commented Feb 8, 2024

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);
}

@cksac cksac reopened this Feb 8, 2024
@dhardy
Copy link
Member

dhardy commented Feb 8, 2024

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.

@dhardy dhardy closed this as completed Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants