-
Notifications
You must be signed in to change notification settings - Fork 179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rounding errors for decimals without a finite representation #414
Comments
I suppose if we always lose the last digit then we'd be avoiding all the rounding errors... I suspect other libraries might simply restrict the max. number of significant digits to one fewer than the max? |
Just to add: To be quite honest, handling rational numbers is out of scope of the library as it stands. i.e. we handle rational numbers to the best of our ability to a precision of 28. For example, the same issue would exist with I wouldn't be opposed to another crate extending features for rational support though (e.g. |
It looks like some of the other libraries handle the inverse logic differently indicating a significant figure rounding. e.g. if we were to use the multiplication example: Python:
Decimal:fn main() {
use decimal::d128;
println!("{}", d128!(3) * (d128!(1) / d128!(3)))
} outputs: Big Decimalfn main() {
use bigdecimal::BigDecimal;
println!("{}", BigDecimal::from(3) * (BigDecimal::from(1) / BigDecimal::from(3)));
} outputs: |
Hit "comment" too early... 😂 What I was going to suggest is that perhaps by implementing #389 we could cater for similar behavior using an opt in approach. i.e. It seems like they're handling the base decimal similarly - however are rounding differently. Of course, the only precise answer is to implement rational numbers but in lieu of that, significant figure rounding may be suitable? |
I may say, if the library only guarantees 28 digits of precision, we should always round the result to 28 digits. The A 96-bit mantissa can only hold 28 full decimal digits (the 29th one incomplete), meaning that numeric results should always normalize to 10^28, throwing out the remainder (if <= 4) or adding one (if >= 5). Essentially rounding the mantissa to 28 digits. If we always normalize all results, then comparisons and display no longer will have this problem. |
I think there is some confusion about the max precision being 28. It doesn't limit the number of digits being used but rather constrains the equation:
Where The example in question is still valid since it is within these constraints. Ultimately it has a mantissa of
Ultimately, because the mantissa is 96 bits, it can have more than 28 digits (base 10) representing it. i.e. The maximum represented value is Regarding normalization: the library already does try to round up/down to ensure it fits within those constraints (e.g. see Line 593 in 3c0f76b
Just to elaborate: the reason the result ends up the way it is is because of:
It's not that the answer is incorrect, it's simply that information is lost on the first calculation. Having the library make the assumption that the number needs to be further rounded feels a little bit risky. My gut is to recommend explicit rounding so that it is clear what is happening within the constraints of the library. An accurate approach of course is to handle rational numbers explicitly. That particular approach could be achieved by |
I think that's the main problem, because 2^96 as a mantissa cannot represent 29 full digits (one of the digits is at most 7, missing 8 and 9). Therefore, you technically cannot represent exact precision of all 29-digit numbers. Which then leads to the fact that, in any calculation, the 29th digit will always be suspect due to imprecise precision. Thus using a 96-bit mantissa gives you either: a 28-digit precise number, or... a 29-digit number which has only 28 digits if it starts with 8 or 9...
I agree the answer is not incorrect. It is correct to the extent of the last digit of precision, as we can see.
This sounds like be a good way to handle this. Explicitly rounding a number to 28 digits will normalize any number, so if this step is done to the calculation result (and before any result comparison), then precise equality should be guaranteed. So the test validations should be revised to: fn main() {
use rust_decimal::Decimal;
let expected = Decimal::new(3,0);
let actual = Decimal::from(1) / (Decimal::from(1) / Decimal::from(3));
let normalized = actual.round_dp_with_strategy(27, MidpointAwayFromZero);
assert_ne!(expected, actual);
assert_eq!(expected, normalized);
} I would suggest adding a |
Got it, I see what you mean - that makes sense.
There is a |
I agree that normally this would not be a problem to most actual use cases... it would just look annoying to people who dislikes seeing On the other hand, however, if a user program actually depends on the fact that So, in other words, it is probably prudent to do this just in case programmers think |
Thank you both for your detailed responses, I am grateful for your time! I realise now that I have misunderstood decimal floating point arithmetic. I had thought the whole reason to use crates like these was to avoid rounding errors Indeed, from the readme of this repo:
However, reading further from the python stdlib and Knuth have made it clear that we can't ever escape rounding errors, just representation errors; you can only be precise up to a finite number of digits. That misunderstanding cleared up, it is worth noting that Java's BigDecimal will actually throw an exception if you ask for an exact result but the result has an inifinite decimal representation. This prevents any surprises like this!
I agree - it might be worth a disclaimer/warning in the readme/docs that this crate/all decimals are precise only to x many digits, and therefore unexpected results like this may occur. I'm also not sure that rounding to 28 digits will always work; consider 1 / (1 / 11000), which unrounded is |
Actually speaking, Once you have that, it is impossible not to have rounding errors. Since infinite representations can only be created via divison, so if you don't have divison, you won't have errors. Of course, you only have max 28 exact digits. However you can get more digits by moving to more bits. But once you have infinite digit numbers, more bits will not help. |
Summary
rust-decimal
has rounding errors for calcluations involving numbers that do not have finite decimal representations. Other decimal libraries do not have similar issues.rust-decimal
I am having an issue with rounding errors. As a simple example, calculating
1 / (1/3)
gives an incorrect result. Code to reproduce the issue:results in:
Replacing
1/3
with any repeating decimal will display the same issue.Similarly, calculating
1 / (1/e)
displays a rounding error. This would suggest that the issue is for any decimal without a finite representation.Comparison with other decimal implementations
Other implementations of decimals give correct results:
BigDecimal crate
BigDecimal code that passes:
Decimal crate
Decimal code that passes:
Python standard library
Python standard library:
Fix
I have not compared the code of the libraries above, so I am not sure where
rust-decimal
's behaviour differs. However, the fact that they all work correctly suggest that this isn't an intractable problem. Happy to help however I usefully can.The text was updated successfully, but these errors were encountered: