dnsl48 / fraction

[Rust] Lossless fractions and decimals; drop-in float replacement
Apache License 2.0
83 stars 25 forks source link

Using `calc_precision` on `1 / 3` does not terminate #34

Closed hydrolarus closed 3 years ago

hydrolarus commented 3 years ago

fraction version: 0.6.3

Problem

For a project that uses fraction I want to implement scientific notation printing of numbers. Sometimes get_precision() does not reflect the actual precision currently used (for example 1 / 100 vs from_decimal_str("0.001")), but using calc_precision() updates this. However this function never terminates for 1 / 3.

use fraction::BigDecimal;

fn main() {
    let x = BigDecimal::from(1) / BigDecimal::from(3);
    dbg!(x.calc_precision());
}
dnsl48 commented 3 years ago

Hi, thanks for reporting that!

Sometimes get_precision() does not reflect the actual precision currently used

That's a utility function that merely returns the current precision which the Decimal instance carries internally. That precision is usually calculated on initialisation and is mainly used for representation purpose (not for calculations). E.g. for formatting functions to decide how many digits to show.

using calc_precision() updates this. However this function never terminates for 1 / 3.

Yes, this function's purpose is to calculate the precision, but for irrational numbers that obviously not going to work. I guess one way to address that would be to add an optional argument to calc_precision, that would limit the max possible precision. If you don't have any other ideas how to approach that, I guess we could roll out a patch to address this. PRs are welcome, but if you don't have time, I'll make a patch sometime this weekend.

dnsl48 commented 3 years ago

However this function never terminates for 1 / 3

Digging a bit deeper into the code, I found out what exactly is happening there. In fact, it terminates, because it performs checked math in there https://github.com/dnsl48/fraction/blob/master/src/decimal/mod.rs#L813

However, the problem is that for BigDecimal we use usize to store precisions, and this type has too high max value. The computation just takes forever. That does not happen for simple Decimal which uses u8 to represent precision, so calc_precision just returns 255 for irrationals.

I'm preparing a patch so that calc_precision can receive a limiting max_precision argument. If None is given, then it would keep calculation until it reaches the exact precision or the capacity of the precision type (same what it does right now).

dnsl48 commented 3 years ago

@tiatomee thanks for reporting that again.

I've published the release 0.7.0 that includes the patch for calc_precision. I also added some more docs for the Decimal precision methods. Hope this helps. If not, please feel free to reopen this issue (or create another one).

hydrolarus commented 3 years ago

That works perfectly fine, thank you very much! :+1: