paupino / rust-decimal

Decimal number implementation written in pure Rust suitable for financial and fixed-precision calculations.
https://docs.rs/rust_decimal/
MIT License
1.02k stars 183 forks source link

I want arithmetic ops which return error instead of losing precision (was: I want arithmetic ops which never lose precision) #515

Closed safinaskar closed 9 months ago

safinaskar commented 2 years ago

I use rust-decimal for monetary calculations. rust-decimal sometimes lose precision, and I don't like that. For example, 1.0000000000000000000000000001 * 1.0000000000000000000000000001 is 1.0000000000000000000000000002 under rust-decimal (this is incorrect from mathematical point of view). (I used 1.23.1.) Even checked_mul gives same answer instead of returning error. I want some function (say, exact_mul and similar names for addition and subtraction), which returns Ok (or Some) when a result can be exactly represented and Err (or None) if not.

In my program (assuming my program is bug-free) such precision losing should never happen. But as we all know we can never be sure that there is no bugs! So I want this exact_mul. I would replace my checked_mul with exact_mul (with unwrap). And if I ever lose precision, I will discover this, because I will see panic

schungx commented 2 years ago

In this case this is the wrong library for you. rust-decimal is not an infinite-precision library. It is fixed-precision, only with a large number of significant digits (I believe up to 96 bits). However, you will run out of precision.

An arbitrary precision numeric library is what you want - but you need to be careful. Some numbers (even rational numbers) cannot be represented in a finite string of decimal digits (e.g. 1/3, PI). And computers have finite memory. Therefore you physically CANNOT have zero loss of precision. If you think so, you're deluding yourself.

Therefore, the conclusion is: if you're going to lose precision anyway, what you really need is to restrict the amount of precision loss and make sure your loss is small enough to be tolerated.

Just to reemphasize: THERE IS NO "NEVER LOSE PRECISION". It is impossible.

You can only get: Lose very very very little precision, so small that it doesn't matter to anybody.

safinaskar commented 2 years ago

rust-decimal is not an infinite-precision library

@schungx . I want fixed precision library, not infinite precision one. I don't want to always retain full precision (edit: I don't want to have arbitrary big precision). I simply want a library to report error when the library would lose precision. (I just changed bug title to reflect this.) Infinite precision libs are too slow for me.

make sure your loss is small enough to be tolerated

I think I was clear enough in bug description. My application has zero tolerance to precision loss. My application deals with money. Its only purpose is to verify that two given monetary amounts are absolutely equal. But I can tolerate panics, assertion faults, etc. They are okey. If I will encounter panic, I will simply dig into data and try to understand why the panic happened and try to do something. Panics are better that silent precision loss I will never know about

safinaskar commented 2 years ago

Again: I want function, which tries to multiply values. When I give 1.0000000000000000000000000001 and 1.0000000000000000000000000001 to this function, it should return Err

schungx commented 2 years ago

In other words, you want methods to fail on overflow or underflow, not rounding.

I suppose the checked_ versions should do this? Not sure why checked_mul doesn't...

This is checked_mul:

    pub fn checked_mul(self, other: Decimal) -> Option<Decimal> {
        match ops::mul_impl(&self, &other) {
            CalculationResult::Ok(result) => Some(result),
            CalculationResult::Overflow => None,
            _ => None,
        }
    }

As you can see, it is supposed to return None upon overflow.

safinaskar commented 2 years ago

@schungx . As I said earlier checked_mul returns Some in my case. Here is my test code:

fn main() {
    use rust_decimal::Decimal;
    let a: Decimal = Decimal::from_str_exact("1.0000000000000000000000000001").unwrap();
    let b = a.checked_mul(a);
    let c = b.unwrap(); // XXX
    assert_eq!(c, Decimal::from_str_exact("1.0000000000000000000000000002").unwrap());
}

This code passes without panic (I use rust_decimal 1.23.1). But I want this code to panic at line marked with XXX

schungx commented 2 years ago

Just a hunch: rust-decimal I believe has 27 digits of precision (or something like that). Your example has 29 digits. Therefore, it cannot be represented by a Decimal in this library. Therefore, a is already inexact.

Technically, you may actually want Decimal::from_str_exact to return None for your input.

safinaskar commented 2 years ago

@schungx . No, a is exact. When I add println!("{}", a) to my code, I see 1.0000000000000000000000000001

schungx commented 2 years ago

No it isn't. It is complicated. Let me explain:

2^96 = 79228162514264337593543950336

You can see MAX-96-bits has 29 decimal digits. However, the first digit can no more than 7, therefore it is missing 8 and 9.

Therefore 96 bits can at most represent 28 digits fully, or 29 digits partially.

Notice the word "partially". It means that a 29-digit number is inexact. You just happen to get the same display.

When you do arithmetic calculations, especially multiplication, that inexactness at the 29th digit will start accumulating errors.

Try your test with only 28 digits and see...

safinaskar commented 2 years ago

Okey, I removed one zero. Result is same. I. e. the following code passes to end instead of panicking.

fn main() {
    use rust_decimal::Decimal;
    let a: Decimal = Decimal::from_str_exact("1.000000000000000000000000001").unwrap();
    let b = a.checked_mul(a);
    let c = b.unwrap();
    assert_eq!(c, Decimal::from_str_exact("1.000000000000000000000000002").unwrap());
}
schungx commented 2 years ago

Well there goes the idea then.

Nevertheless, checked_mul should easily see that both operands have scale of 28 and more, so there is a high potential for the result to overflow. Not sure why it doesn't return None in this case.

Maybe @paupino can take a look at this, as the checked_mul code is quite hairy...

paupino commented 2 years ago

This is actually the same issue as #511 which got raised the other day too.

Effectively, what we're talking about here is underflow handling. By default, rust_decimal will attempt to round underflow if it can - this is typically a useful feature, for example 1 / 3 is difficult to represent (without storing ratios) so we instead try to "round off" the underflow to "make it fit".

Furthermore, multiplication is a bit different to other operations. While it's true that we reserve 96 bits for the mantissa representation within a standard decimal, we effectively reserve 192 bits for the product. This is because multiplication can naturally increase the scale - e.g. 1.1 * 1.1 = 1.21. To convert it back to 96 bits we need to (if we can) scale back and/or round to make the underflow fit.

The checked_ functions currently handle the overflow cases. This is by design since underflow isn't typically an error case. That being said, I can understand the want/desire to maintain underflow precision and know if it is in fact unable to be represented without rounding/scaling.

There are a couple of ways for going about this. The first is to modify the bitwise functions to make underflow handling optional. This could then be exposed either via a feature flag or an explicit function. My concern with this approach is that it is very limited in scope - it's relatively easy to underflow, and sometimes in ways that have no meaningful difference (e.g. 1.00000000000000000000 * 2.00000000000000000000 technically underflows).

The second is to provide "delayed boxing" functionality. That is, keeping the number in it's raw Buf24 state (or whatever) until time to "evaluate" the Decimal. This would technically allow some operations that would underflow to be potentially recovered (e.g. via a round). The reason I like this approach slightly more is because there are times that you want to maintain a high precision until the very end (e.g. powd).

Anyway, all this to say - there isn't a "quick fix" to this right now - it's currently working by design. It's on my todo list to take a look at the fundamentals of rust_decimal in prep for v2 (i.e. alternative storage formats as well as some of the features we talked about) however that's still a while away.

I will go on to say that bigdecimal.rs may be more appropriate for your current use case if performance isn't a concern since that effectively uses a BigInt behind the scenes and allows for a much higher scale.

schungx commented 2 years ago

Make sense! Underflow means very very small errors (errors that are smaller than the minimum number representable by this format) and in most usage you'd want to ignore it (therefore rounding it up).

However, it may disrupt the meaning of checked_xxx where the user may not think that any error has occurred if the result is Some. Maybe, as the OP asks, an exact_xxx that returns None upon underflow instead of rounding is a simple resolution?

safinaskar commented 2 years ago

@paupino Thanks for answer. I simply wanted to know whether this feature will be added in reasonable time or I should simply write my own lib. So it seems the latter is true. Okey. (And in fact I need decimal fixed point, not decimal floating point, so writing my own lib should be easy.)

I will go on to say that bigdecimal.rs may be more appropriate for your current use case

You mean https://docs.rs/bigdecimal ? I think it is slow, I don't like this. Moreover, it seems to be badly designed, I wonder is there at least one use case, where bigdecimal will be actually useful. Let me show you example of bad design. Consider this pseudo code:

// Pseudo code! Uses bigdecimal
let a = from_str("0.5") + from_str("0.5");
loop {
  let b = a * a;
  a = b;
}

This code should loop forever and every iteration should be fast (because we simply do 1 * 1 at every iteration). But in fact every iteration turns to be 2x slower than previous. (I will report this to upstream tomorrow with actual code example.)

safinaskar commented 2 years ago

So I will go writing my lib. You may close the bug

paupino commented 2 years ago

You mean https://docs.rs/bigdecimal ?

Yep, that's the one I was referring to!

I simply wanted to know whether this feature will be added in reasonable time or I should simply write my own lib. So I will go writing my lib. You may close the bug

Adding this feature is definitely on the roadmap as it has come up a few times however when is still open for discussion. I'd like to take a look at this (and surrounding issues) this month however it all depends on how my work schedule pans out! I'll keep this issue open for the meantime as it helps me also group by demand.

If you did want to have a go at adding the feature instead of writing a new lib then then branching logic for mul underflow is here:

https://github.com/paupino/rust-decimal/blob/master/src/ops/mul.rs#L132

The rescale function that does the actual rescaling/rounding is here:

https://github.com/paupino/rust-decimal/blob/master/src/ops/common.rs#L337

Either way: good luck and thanks for creating an issue!

safinaskar commented 2 years ago

I reported bug I mentioned: https://github.com/akubera/bigdecimal-rs/issues/86

Tony-Samuels commented 9 months ago

Duplicate of #511. We'll keep the other one open for now, as this one seems to be resolved by just switching to bigdecimal.