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

Multiply looses precission #176

Closed
lhoracek opened this issue May 9, 2021 · 10 comments
Closed

Multiply looses precission #176

lhoracek opened this issue May 9, 2021 · 10 comments
Assignees

Comments

@lhoracek
Copy link

lhoracek commented May 9, 2021

Describe the bug
Numbers rounded to specific number of of digits after decimal point multiply incorrectly.

To Reproduce
Steps to reproduce the behavior:

val a = "5.61".toBigDecimal().roundToDigitPositionAfterDecimalPoint( 2, RoundingMode.ROUND_HALF_AWAY_FROM_ZERO)
val b = "2.95".toBigDecimal().roundToDigitPositionAfterDecimalPoint( 2, RoundingMode.ROUND_HALF_AWAY_FROM_ZERO)
val res = a.multiply(b).roundToDigitPositionAfterDecimalPoint( 2, RoundingMode.ROUND_HALF_AWAY_FROM_ZERO)
assertEquals("16.55", res.toStringExpanded())
expected:<16.5[5]> but was:<16.5[]>
Expected :16.55
Actual   :16.5

Expected behavior

Result should not loose precision same way java BigDecimal.setScale(2, RoundMode.HALF_UP)

BigDecimal bd1 = new BigDecimal("5.61").setScale(2, RoundingMode.HALF_UP); 
BigDecimal bd2  = new BigDecimal("2.95").setScale(2, RoundingMode.HALF_UP); 
BigDecimal bs3 = bd2.multiply(bd1).setScale(2, RoundingMode.HALF_UP); 
String str = "Result is " +bs3;
System.out.println(str); 

Platform
JVM (commonTest run by testDebugUnitTest)

@ionspin
Copy link
Owner

ionspin commented May 9, 2021

Multiplication didn't loose precision, roundToDigitPositionAfterDecimalPoint was setting a DecimalMode to the number of digits in the result after rounding which then caused multiplication to round to that decimal mode, which in this case was 3. Still I feel this was completely unexpected behavior of roundToDigitPosition methods, so now #177 will change the behavior so they only apply decimal mode after rounding if the users sets it explicitly.

@ionspin
Copy link
Owner

ionspin commented May 9, 2021

Oh btw, the fix will be available in 0.3.1-SNAPSHOT as soon as the gitlab build is completed.

@ionspin
Copy link
Owner

ionspin commented May 9, 2021

And also, thanks for reporting!

@lhoracek
Copy link
Author

lhoracek commented May 9, 2021

Will test ASAP, but even when I use val dm = DecimalMode(99999, RoundingMode.ROUND_HALF_AWAY_FROM_ZERO, 2) and create new BigDecimal with fun BigDecimal.round() = BigDecimal.fromBigDecimal(this, dm)

val out = ("5.61".toBigDecimal().round() * "2.95".toBigDecimal().round() * "14.41".toBigDecimal().round()).round()
assertEquals("238.48", out.toStringExpanded())
expected:<238.4[8]> but was:<238.4[9]>
Expected :238.48
Actual   :238.49

Are my expectations wrong here? The actual result is 238,478295 .. or is this cause by the actual overloaded operator, that does the rounding after each of those operations?

@lhoracek
Copy link
Author

lhoracek commented May 9, 2021

Or better question is "how to properly do it with this lib"? I have to admit I've expected just using setScale the same way I did with java's BigDecimal

@ionspin
Copy link
Owner

ionspin commented May 9, 2021

I am not completely sure, I would have to take step through debugger to see what is going on with your sample, but I don't have time right now. Keep in mind that the when using decimal mode with scale, scale overrides the decimal precision so even if you put 9999 the scale 2 is used. DecimalMode API will be changed at some point to be more user friendly, at the moment there are too many pitfalls.

ionspin added a commit that referenced this issue May 9, 2021
…-loss

Fix for #176, changed rounding after digit method behavior
@ionspin
Copy link
Owner

ionspin commented May 9, 2021

Or better question is "how to properly do it with this lib"? I have to admit I've expected just using setScale the same way I did with java's BigDecimal

Your usage DecimalMode(99999, RoundingMode.ROUND_HALF_AWAY_FROM_ZERO, 2) would work for scale 2. Again I'm not sure about the order of rounding you are applying without debugging it.

The original cause why this part of the API is not easy to use is because originally the library used only decimal precision, and scale was a later contribution. I'll probably be dropping the decimal precision from the API in the upcoming versions.

@ionspin
Copy link
Owner

ionspin commented May 9, 2021

From a quick test with the latest with the latest build that should be available in the snapshots this could work for you

        val dm = DecimalMode(99999, RoundingMode.ROUND_HALF_AWAY_FROM_ZERO, 2)
        fun BigDecimal.round() = BigDecimal.fromBigDecimal(this, dm)
        val out = ("5.61".toBigDecimal() * "2.95".toBigDecimal() * "14.41".toBigDecimal()).round()
        assertEquals("238.48", out.toStringExpanded())

@lhoracek
Copy link
Author

lhoracek commented May 9, 2021

Just tested it and it works great. Thanks for the quick fix!

@lhoracek
Copy link
Author

lhoracek commented May 9, 2021

Can be closed or left until new release is done. 👍

@ionspin ionspin closed this as completed May 9, 2021
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