aldanor / fast-float-rust

Super-fast float parser in Rust (now part of Rust core)
https://docs.rs/fast-float
Apache License 2.0
275 stars 20 forks source link

Wrong value with very long floats #4

Closed AnttiParaoanu closed 3 years ago

AnttiParaoanu commented 3 years ago

For some floats with a long trail of zeroes, the value is not correctly rounded after a sufficient amount of zeroes.

Code:

use fast_float::*;

fn main() {
    let base = "1.1754941406275178592461758986628081843312458647327962400313859427181746759860647699724722770042717456817626953125";
    let mut a = String::from(base);
    let mut b = a.clone();
    a.push_str(&"0".repeat(655));
    a.push_str("e-38");
    b.push_str(&"0".repeat(656));
    b.push_str("e-38");
    let a = parse::<f32, &str>(&a).unwrap();
    let b = parse::<f32, &str>(&b).unwrap();
    println!("a: {:x}", a.to_bits());
    println!("b: {:x}", b.to_bits());
}

Output:

a: 7ffffe
b: 7fffff

Works in debug and release modes

rustc --version --verbose

rustc 1.47.0-nightly (6c8927b0c 2020-07-26)
binary: rustc
commit-hash: 6c8927b0cf80ceee19386026cf9d7fd4fd9d486f
commit-date: 2020-07-26
host: x86_64-unknown-linux-gnu
release: 1.47.0-nightly
LLVM version: 10.0
aldanor commented 3 years ago

@gretingz Thanks for reporting, will take a look. It's the Decimal::round() and its truncated flag basically which in cases like this shouldn't be taken into account.

lemire commented 3 years ago

This issue seems to affect that C++ code as well (verified).

aldanor commented 3 years ago

In this particular case, I think it can be solved like this: tracking number of significant digits (along the total number of digits) when parsing a decimal. Then, in this case, truncated flag won't be set it all.

Is there anything else though?

lemire commented 3 years ago

@aldanor My expectation is that you are correct. I just wanted to point out that it is likely your inherited this bug from me. Tracing bugs is important!

lemire commented 3 years ago

The fix is trivial as you have no doubt realized. Here is what I added in C++:

  if(answer.num_digits > 0) {
    // We potentially need the answer.num_digits > 0 guard because we
    // prune leading zeros. So with answer.num_digits > 0, we know that
    // we have at least one non-zero digit.
    const char *preverse = p - 1;
    int32_t trailing_zeros = 0;
    while ((*preverse == '0') || (*preverse == '.')) {
      if(*preverse == '0') { trailing_zeros++; };
      --preverse;
    }
    answer.decimal_point += int32_t(answer.num_digits);
    answer.num_digits -= uint32_t(trailing_zeros);
  }

This is not the prettiest code, but basically, I run in reverse, counting the number of trailing zeroes and I subtract it from the number of digits, which corrects the truncated check later.

lemire commented 3 years ago

@aldanor To reproduce it with 64-bit floats, use 9007199254740993.0 and then append many zeros to trigger the truncated flag.