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

different behavior of to_f32 and to_f64 #53

Open
Shoeboxam opened this issue Aug 29, 2024 · 5 comments
Open

different behavior of to_f32 and to_f64 #53

Shoeboxam opened this issue Aug 29, 2024 · 5 comments

Comments

@Shoeboxam
Copy link

I just wanted to check if it is intentional to have to_f32 respect the rounding mode of self, but to_f64 always round to HalfEven. I think the API would be less surprising if these behaved the same way, and more useful if to_f64 were adjusted to use the same rounding mode as self.

Thanks as always!

@Shoeboxam
Copy link
Author

Shoeboxam commented Aug 29, 2024

On further testing, it seems to_f32 doesn't actually respect the rounding mode of self:

let min = FBig::<Up>::try_from(f32::from_bits(1))?;
let half: FBig::<Up> = min / 2;
println!("{}", half);
// 0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001
println!("{}", half.to_f32().value());
// 0

Aside from the documentation fix, is it possible to convert back to native types with controlled rounding? I guess I could increment/decrement the resulting float based on the reported rounding mode.

@cmpute
Copy link
Owner

cmpute commented Aug 30, 2024

The intended effect of to_f32 and to_f64 is to respect its own rounding mode (not always HalfEven). If not, then it's a bug. The example you provided is kind of the corner case, where the float is the least subnormal float

@cmpute
Copy link
Owner

cmpute commented Aug 30, 2024

Ensuring rounding works correctly for subnormals is challenging, could you provide some more test cases? I can take a look, but I can't assure a timeline to fix it..

@Shoeboxam
Copy link
Author

No problem! The software I'm writing tends to seek out and find these edge-cases: lots of binary searches for extreme points. This is good for dashu, because you're getting more test coverage indirectly, but it's a bit challenging for us (although still better than getting reliable MPFR builds on windows platforms). In this case, the incorrect rounding causes another computation that should return infinity to instead return ~9. This is where it under-flows and triggers the incorrect subnormal behavior.

The implementations themselves are different: the f64 method makes weaker promises by only rounding to HalfEven, so it isn't "wrong". The f32 method makes more useful promises, but doesn't always uphold them, at least in this case.

#[test]
fn test_subnormal() {
    let min = FBig::<Up>::try_from(f32::from_bits(1)).unwrap();
    let half: FBig::<Up> = min / 2;
    assert!(half.to_f32().value() > 0.);
}

Note that in the above test, the output is Approximation::Inexact(0.0, NoOp). I'm not really sure how to interpret what no-op means. I'd rather always know if the in-exactness errs up or down.

Don't get me wrong, this library is incredibly useful! I provide bug reports whenever I encounter issues because I know it will help make dashu more robust. I understand you're probably busy with other things as well.

Shoeboxam added a commit to opendp/opendp that referenced this issue Sep 9, 2024
Fixes #1997

I'd consider this more of a patch than a fix. At some point when Dashu
is updated, one of our tests will start to fail, and we can try to
remove the patch.

cmpute/dashu#53
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

4 participants
@Shoeboxam @cmpute and others