stencillogic / astro-float

Arbitrary precision floating point numbers library
MIT License
101 stars 4 forks source link

`sinh` of a large negative number returns positive infinity #17

Closed benjamin-cates closed 11 months ago

benjamin-cates commented 11 months ago

Hello! I really like this package and I've been working with it a lot, however I have found a bug :(

Reproducible example in astro-float 0.9.1

use astro_float
#[test]
fn test_sinh() {
    let prec = 256;
    let mut cc = astro_float::Consts::new().unwrap();
    let sinh_neg = astro_float::BigFloat::from_f64(-1e50, prec).sinh(
        prec,
        astro_float::RoundingMode::None,
        &mut cc,
    );
    // Passes :(
    assert!(sinh_neg.is_inf_pos());
    // Fails :(
    assert!(sinh_neg.is_inf_neg());
}

Basically the sinh of any really negative number will return positive infinity when it should return negative infinity. However the sinh of negative infinity is correct. I believe this is because the exponent overflow error from 1/(e^x) does not swap signs. Basically if x is really negative, the reciprocal operation on positive zero will overflow to positive infinity and the offending line of code (below) will pass on the positive overflow onto the return value even though it is being subtracted from the final result later.

let xe = ex.reciprocal(p_x, RoundingMode::None)?;

Link to line of code that's broken

It was likely intended that the question mark operator would pass the error into the map_err function but it actually just returns it.

I think the best solution would be to replace the question mark operator with a match statement that just sets the overflow sign to negative (this is probably the fastest because it is doing the match statement anyway).

I am willing to submit a pull request to fix this or if you want to fix it that would be cool too.

stencillogic commented 11 months ago

Hello, thank you for raising the issue.

It was likely intended that the question mark operator would pass the error into the map_err function but it actually just returns it.

Yes, I agree. It looks like a misuse of ? operator. PR is welcome (please don't forget about including tests).