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

Tweak Open01 and Closed01 #237

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
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
35 changes: 32 additions & 3 deletions src/rand_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,38 @@ mod tests {

#[test]
fn floating_point_edge_cases() {
// the test for exact equality is correct here.
assert!(ConstantRng(0xffff_ffff).gen::<f32>() != 1.0);
assert!(ConstantRng(0xffff_ffff_ffff_ffff).gen::<f64>() != 1.0);
const EPSILON32: f32 = 1.0 / (1u32 << 23) as f32;
const EPSILON64: f64 = 1.0 / (1u64 << 52) as f64;
Copy link
Member

Choose a reason for hiding this comment

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

Why not use core::f32::EPSILON?

ε is defined as the difference between 1.0 and the next largest representable number; since all numbers output (excepting Closed01) are strictly less than 1.0 the exponent will be 1 less than for 1.0, hence the largest representable number less than 1 should be 1 - ε / 2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I first used std::f32::EPSILON but that failed the continuous integration tests and I forgot about core, so I'll change this.


let mut zeros = ConstantRng(0);
let mut ones = ConstantRng(!0);

let zero32 = zeros.gen::<f32>();
let zero64 = zeros.gen::<f64>();
let one32 = ones.gen::<f32>();
let one64 = ones.gen::<f64>();
assert_eq!(zero32, 0.0);
assert_eq!(zero64, 0.0);
assert!(1.0 - EPSILON32 <= one32 && one32 < 1.0);
assert!(1.0 - EPSILON64 <= one64 && one64 < 1.0);
Copy link
Member

Choose a reason for hiding this comment

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

As noted above, I believe this should be exactly 1 - ε / 2. Exact equality tests here would be preferable (excepting the Open01 case near 0, where there are many near representable values).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, one32 should be 1.0 - f32::EPSILON, not 1.0 - f32::EPSILON / 2, since the generator generates a multiple of ɛ in gen::<f32>. If we are to use the exact equality tests, they should be (zero32, one32) == (0.0, 1.0 - f32::EPSILON), (closed_zero32, closed_one32) == (0.0, 1.0) and (open_zero32, open_one32) == (f32::EPSILON / 2, 1.0 - f32::EPSILON / 2). Should I amend the commit to do that?


let Closed01(closed_zero32) = zeros.gen::<Closed01<f32>>();
let Closed01(closed_zero64) = zeros.gen::<Closed01<f64>>();
let Closed01(closed_one32) = ones.gen::<Closed01<f32>>();
let Closed01(closed_one64) = ones.gen::<Closed01<f64>>();
assert_eq!(closed_zero32, 0.0);
assert_eq!(closed_zero64, 0.0);
assert_eq!(closed_one32, 1.0);
assert_eq!(closed_one64, 1.0);

let Open01(open_zero32) = zeros.gen::<Open01<f32>>();
let Open01(open_zero64) = zeros.gen::<Open01<f64>>();
let Open01(open_one32) = ones.gen::<Open01<f32>>();
let Open01(open_one64) = ones.gen::<Open01<f64>>();
assert!(0.0 < open_zero32 && open_zero32 <= EPSILON32);
assert!(0.0 < open_zero64 && open_zero64 <= EPSILON64);
assert!(1.0 - EPSILON32 <= open_one32 && open_one32 < 1.0);
assert!(1.0 - EPSILON64 <= open_one64 && open_one64 < 1.0);
}

#[test]
Expand Down