ionspin / kotlin-multiplatform-bignum

A Kotlin multiplatform library for arbitrary precision arithmetics
Apache License 2.0
372 stars 42 forks source link

RoundingMode ROUND_HALF_AWAY_FROM_ZERO is not returning the expected value #313

Closed YoannMeste closed 3 weeks ago

YoannMeste commented 1 month ago

When performing a simple division 2/3 with RoundingMode.ROUND_HALF_AWAY_FROM_ZERO and a scale of 2, I expect the result to be 0.67. However, I get 0.66 instead.

Steps to Reproduce

BigDecimal.fromInt(2)
    .divide(BigDecimal.fromInt(3), DecimalMode(2, RoundingMode.ROUND_HALF_AWAY_FROM_ZERO))
    .doubleValue(false)
// returns 0.66

// In contrast, using java.math.BigDecimal:
java.math.BigDecimal.valueOf(2).divide(java.math.BigDecimal(3), 2, java.math.RoundingMode.HALF_UP).toDouble()
// returns 0.67

I expected to obtain the same result as java.math.BigDecimal.

Is my understanding of this rounding mode incorrect?

ionspin commented 1 month ago

Sounds reasonable at first glance, I'll take a look at it when time permits, thanks for reporting!

YoannMeste commented 3 weeks ago

I had a look at the code, it seems to be decided by this portion of code

        private fun determineDecider(discarded: BigInteger): SignificantDecider {
            val scale = (BigInteger.TEN.pow(discarded.numberOfDecimalDigits() - 1))
            val divrem = discarded.divrem(scale)
            val significant = divrem.quotient.abs().intValue(true)
            val rest = divrem.remainder.abs()
            return when {
                significant == 5 -> {
                    if (rest == BigInteger.ZERO) {
                        SignificantDecider.FIVE
                    } else {
                        SignificantDecider.MORE_THAN_FIVE
                    }
                }
                significant > 5 -> SignificantDecider.MORE_THAN_FIVE
                significant < 5 -> SignificantDecider.LESS_THAN_FIVE
                else -> throw RuntimeException("Couldn't determine decider")
            }
        }

If my understanding is correct it should return MORE_THAN_FIVE but in our case it returns LESS_THAN_FIVE

I suspect the value 5 in the condition >5 but i am not so sure about that. What do you think ?

ionspin commented 3 weeks ago

Thanks for looking in to it, you are probably on the right track.

I'm in meetings whole day today so I'll have a look at it tomorrow or over the weekend and get back to you so we can fix this.

ionspin commented 3 weeks ago

You were close to the problem, it was one step above, the discarded value was incorrect, as it was just the remainder of the division, instead of the actual discarded digits. The fix should be pushed soon. Once again thanks for reporting and narrowing down the issue!