This f64 value round-trips with 1 ULP error

[alexhuszagh]

This f64 value round-trips with 1 ULP error

[dangrabcad]

Thanks for fixing this! I'm glad my bug report was so useful to you. It was really easy to find this edge case using proptest: it was the second or third test run that found it. If you like, I'd be happy to set up proptest to test lexical and lexical-core, and send you a PR. That way it should be easy to find any more values like this lurking in the corners.

[plokhotnyuk]

@Alexhuszagh I'm happy that the issue was caught and fixed so fast!

I've checked this case in a simplified Scala implementation of your algorithm for base 10 and found that it successfully passes through the medium path that uses a big lookup table (653 values of 64-bit coefficients) and one operation of 64-bit multiplication of mantissa.

Have you considered providing the specialized implementation for base 10?

For cases when you need to parse 64-bit float values that were previously serialized from the binary representation, it allows minimizing the need of subsequent slower paths down to 0.22% (for arbitrary binary representation).

[Alexhuszagh]

Thank you for finding this, let me first confirm the issues compared to other parsers, and I'll triage it accordingly.

[Alexhuszagh]

@dangrabcad Confirmed, it's a core error in the slow-path algorithm used by default, let me check if this exists in other versions.

Specifically, bhcomp is using a b value of 7.689539722041642e164, and is using the internal routine small_atof to generate the significant digits. This is an amazing catch, thank you for finding this.

The core bug seems to be in small_atof in bhcomp.rs, which is giving me a value lower than the compared digits for real_digits, so the comparison suggests that we should round-down, rather than round-up, maybe due to an overflow or underflow during the correct scaling. Quite a nice find, thank you.

[Alexhuszagh]

@dangrabcad I've confirmed the source of the issue, the scaling in small_atof is exactly 10 off for the real digits as opposed to the b+h digits, which is likely due to calculating the exponent in this case off-by-1 exactly. Should be an easy patch.

[Alexhuszagh]

@dangrabcad The integration tests should fail (an unrelated bug I need to patch), but the following should fix the error. I've diagnosed the source of the issue, and it's a rather larger bug than I was hoping, and a very good catch. I'll add in more unittests to ensure this is never an issue again.

In short, any floating point-number with trailing 0s in the fraction part that forced the slow path (IE, more than 17 digits and near a halfway point) may have been rounded incorrectly. I'll be updating the README accordingly, crediting you for finding this, and adding in further tests to ensure this never happens again. I cannot thank you enough.

After patching the various dependency issues on older Rust versions (the issue with the integrations), I should push a commit later today.

Once again, thank you. Quite a catch.

[Alexhuszagh]

Fixed as of v0.4.3.

[Alexhuszagh]

@dangrabcad I already have some integrations for proptest, but clearly they aren't comprehensive enough. Please send over anything you have. This discovery sqaushed a class of bugs, and was beyond useful.

Also, @plokhotnyuk, I've been fairly swamped lately, but I would to. Also, I've been very glad my work has been useful to more than just the Rust ecosystem.