Skip to content
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

Scale depends on computing order of additions when subsequent result is zero #695

Open
35VLG84 opened this issue Dec 25, 2024 · 4 comments

Comments

@35VLG84
Copy link

35VLG84 commented Dec 25, 2024

Hello,

Decimal's scale depends on computing order of additions, if additions subsequent result is zero:

#[test]
fn test_scale_with_add() {
    let d3 = dec!(3.001);
    let d2 = dec!(2.01);

    let d_332 = -d3 + d3 + d2;
    let d_323 = -d3 + d2 + d3;
    assert_eq!(d_323, d_332);
    assert_eq!(d_323.scale(), 3);

    // This assert fails:
    assert_eq!(d_323.scale(), // scale = 3
               d_332.scale()); // scale = 2
}

And one test to check that scale is preserved in normal case:

#[test]
fn test_preserve_scale() {
    let d9 = dec!(9.000000000);
    let d3 = dec!(3.000);

    assert_eq!(d9.scale(), 9);
    assert_eq!(d3.scale(), 3);

    let d = d9 + d3;
    assert_eq!(d.scale(), 9);
}
35VLG84 added a commit to e257-fi/tackler-tests that referenced this issue Dec 25, 2024
Balance and Balance-Group need own references for:
paupino/rust-decimal#695

Signed-off-by: 35V LG84 <[email protected]>
35VLG84 added a commit to e257-fi/tackler-ng that referenced this issue Dec 25, 2024
Balance and Balance-Group need own reference vectors for:
paupino/rust-decimal#695

Signed-off-by: 35V LG84 <[email protected]>
@35VLG84 35VLG84 changed the title Scale depends on computing order of additions Scale depends on computing order of additions when subsequent result is zero Dec 26, 2024
@Tony-Samuels
Copy link
Collaborator

Is this behaviour causing an issue?

@35VLG84
Copy link
Author

35VLG84 commented Jan 13, 2025

Hi,

Yes, zeros after a decimal (e.g., 1.0) are significant numbers, and this is causing loss of information. Also it is inconsistent behavior as it depends on computing order: sometimes you have all significant numbers, sometimes not.

Concrete issue is that this is breaking a test system, which is validating presence of all significant figures.

@Tony-Samuels
Copy link
Collaborator

Tony-Samuels commented Jan 13, 2025

Our PartialEq implementation ignores the scaling for a reason. The scale does not indicate the maximum number of significant figures in your calculation chain, just for the current value. This is also why you end up with a different answer.

You can see that this is almost impossible to guarantee the scale between different calculation ordering with a simple case like 1 / 2 * 2 (which goes via 1 decimal place) vs 1 * 2 / 2 (which doesn't require any decimal places).

Whilst you're considering addition and subtraction instead of multiplication, the underlying principle applies. If you want to check equality, use the existing functions for doing so.

@35VLG84
Copy link
Author

35VLG84 commented Jan 14, 2025

Hi, this is not about PartialEq failing (the scale is not used for eq comparisions), and this is also not about division, where there are impossible cases like 1 / 3.

This is simply about inconsistent behavior with additions and scale. The bug happens when additions and subtraction are done so that chained temporary result is zero. However, if the end result is zero, then it works just fine, which is odd.

Mathematically this should preserve max scale, and also rust-decimal is preserving it most of the time.

Please see test cases below, which demonstrates this inconsistent behavior:

#[test]
fn test_scale() {
    // Scale works with high + low
    let d1 = dec!(9.000000000) + dec!(3.000);
    assert_eq!(d1.scale(), 9);

    // Scale works with low + high
    let d2 = dec!(3.000) + dec!(9.000000000);
    assert_eq!(d2.scale(), 9);

    // Scale works when result is zero
    let d3 = dec!(3.000) + dec!(-3.000);
    assert_eq!(d3.scale(), 3);

    // Scale works with mixed scale
    let d4 = dec!(3.000) + dec!(2.01) +dec!(-3.000);
    assert_eq!(d4.scale(), 3);

    // Scale works also with this
    // (the computational temp result is not zero)
    let d5 = dec!(3.000) + dec!(-4.000) + dec!(2.01);
    assert_eq!(d5.scale(), 3);

    // Scale doesn't work, when computational temp result is zero
    // However, in above case 3.000 + -3.000 scale worked
    let d6 = dec!(3.000) + dec!(-3.000) + dec!(2.01);
    assert_eq!(d6.scale(), 3); // <--- this assert fails: 2 != 3
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants