Incorrect results of calculations

[avently]

Describe the bug
Hello. I'm migrating to Kotlin/Native and use your BigNum library as actual declaration for BigDecimal. on JVM I have Java's BigDecimal. Many of my tests are failing because the result of calculation on Native with your lib is different.
I provide some examples. Since the migration to native is a long process I can't know all possible errors with the calculations because I comment failing tests one-by-one. The more I comment, the more errors I see. If I may I'll add more errors later.

To Reproduce
All this values are not equal to what your lib show. So you can just compare both libs and find a cause of different result.

println(BigDecimal("1").divide(BigDecimal("110"), 15,RoundingMode.CEILING ).toPlainString())

println(BigDecimal("6075.6").divide(BigDecimal("0.5"), 0,RoundingMode.CEILING ).toPlainString())

println(BigDecimal("6075.7").divide(BigDecimal("0.5"), 0,RoundingMode.CEILING ).toPlainString())

println(BigDecimal("6075.8").divide(BigDecimal("0.5"), 0,RoundingMode.CEILING ).toPlainString())

println(BigDecimal("6075.9").divide(BigDecimal("0.5"), 0,RoundingMode.CEILING ).toPlainString())

println(BigDecimal("0.001294914096").divide(BigDecimal("0.000001"), 0,RoundingMode.CEILING ).toPlainString())

println(BigDecimal("93545.3695").divide(BigDecimal("1"), 0,RoundingMode.FLOOR ).toPlainString())

Another problem I met is that I can't leave decimalPrecision like default value 0 when have a scale specified. Which is strange because default value 0 should be "unlimited precision" as per the docs. I always have to provide it. I think it's not right. I only interested in scale and don't need to limit precision. Right?

Expected behavior
I think the results of the lib should be the same as in Java's BigDecimal

Also there is a difference of how BigNum shows output of toExpandedString in comparison to Java's solution. It always strips zeroes from the end of a string. Is it expected? If so, why? In some places it's useful to have a text like "0.00" which gives a hint on scale of the number shown in UI. Maybe it would be a good idea to show zeroes at the end and make another function like toExpandedWithoutZeroes() or even better to change toPlainString() to show zeroes? That for would be great. Both functions will be useful then. What to you think?

Platform
Linuxx64, JVM

Additional context
Add any other context about the problem here.

[avently]

Maybe would be a good idea to generate millions of random double's, convert them to strings, create Java's bigdecimal instances and BigNum instances and compare them? It will highlight the difference right after start

[avently]

Is there any snapshot build I can use for testing? Because 0.28 version looks outdated
(Ok, using sonatype snapshots)

[ionspin]

Hi @avently,

Thanks for reporting the issue, unfortunately I don't understand fully what seems to be the problem.

I'm migrating to Kotlin/Native and use your BigNum library as actual declaration for BigDecimal.

The BigNum library is a multiplatform library, there is no need to use expect/actual mechanism with it. If you are just using the native artifact to integrate it with your existing api I guess in that case it's fine, but keep in mind that this is not a drop in replacement for JVM BigInteger/BigDecimal, and it was never meant to be so. In this case you will run into problems because the APIs are different!

All this values are not equal to what your lib show. So you can just compare both libs and find a cause of different result.

I can't quite figure out what your example is running, I presume java BigDecimal, I tried one of your examples on java BigDecimal, my BigDecimal on jvm and on native and got the same result .

This was the example I chose (disregarding the unsupported precision 0):
println(BigDecimal("6075.6").divide(BigDecimal("0.5"), 0,RoundingMode.CEILING ).toPlainString())
The result in all three cases was correct, 12151.2

Could you please provide an example using my library and the jvm library where the results are differnt so I can investigate more?

Another problem I met is that I can't leave decimalPrecision like default value 0 when have a scale specified. Which is strange because default value 0 should be "unlimited precision" as per the docs. I always have to provide it. I think it's not right. I only interested in scale and don't need to limit precision. Right?

Originally the library only supported precision, scale was a valued contribution added later, but it was still tied to precision, and that is still true, if you use rounding you need to supply precision. You are completely right that if you define scale you shouldn't have to define precision, and I am aware that API needs improvements here, but I can't give you any timeline as I have limited time to work on adding new features.

A pull request improving the API in this manner would be very welcome.

Also there is a difference of how BigNum shows output of toExpandedString in comparison to Java's solution. It always strips zeroes from the end of a string. Is it expected? In some places it's useful to have a text like "0.00" which gives a hint on scale of the number shown in UI. Maybe it would be a good idea to show zeroes at the end and make another function like toExpandedWithoutZeroes() or even better to change toPlainString() to show zeroes? That for would be great. Both functions will be useful then. What to you think?

Yes it's expected, no particular reason, at the time I was implementing the string functions I didn't see the usefulness of trailing zeroes. Adding a new function that would provide output formatting in the spirit of https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.text/format.html would be cool, once again contributions are highly appreciated!

Maybe would be a good idea to generate millions of random double's, convert them to strings, create Java's bigdecimal instances and BigNum instances and compare them? It will highlight the difference right after start

Unfortunately this is not something I plan on doing, this library was never meant to be a drop in replacement to Java's BigDecimal/BigInteger and as long as results are mathematically correct I am happy. Of course if you want to implement an additional function like toStringWitthJvmFormat() I would love to take that pull request into consideration.

Is there any snapshot build I can use for testing? Because 0.28 version looks outdated

The latest snapshot is 0.3.0, the latest stable release is 0.2.8, and it was released last month, so not completely outdated :)

Once again, thanks for reporting and apologies if I misunderstood anything.

[avently]

Here is a test class you can copy-paste and check:

import com.ionspin.kotlin.bignum.decimal.DecimalMode
import org.junit.Test
import java.math.BigDecimal
import java.math.RoundingMode
import kotlin.random.Random
import kotlin.test.assertEquals
import com.ionspin.kotlin.bignum.decimal.BigDecimal.Companion as BN
import com.ionspin.kotlin.bignum.decimal.RoundingMode as BNMode

class ComparisonTest {
    @Test
    fun compareTest1() {
        assertEquals(
            BigDecimal("1").divide(BigDecimal("110"), 15, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("1").divide(BN.parseString("110"), DecimalMode(100, BNMode.CEILING, 15)).toPlainString()
        )
    }

    @Test
    fun compareTest2() {
        assertEquals(
            BigDecimal("6075.6").divide(BigDecimal("0.5"), 0, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("6075.6").divide(BN.parseString("0.5"), DecimalMode(100, BNMode.CEILING, 0)).toPlainString()
        )
    }

    @Test
    fun compareTest3() {
        assertEquals(
            BigDecimal("6075.7").divide(BigDecimal("0.5"), 0, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("6075.7").divide(BN.parseString("0.5"), DecimalMode(100, BNMode.CEILING, 0)).toPlainString()
        )
    }

    @Test
    fun compareTest4() {
        assertEquals(
            BigDecimal("6075.8").divide(BigDecimal("0.5"), 0, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("6075.8").divide(BN.parseString("0.5"), DecimalMode(100, BNMode.CEILING, 0)).toPlainString()
        )
    }

    @Test
    fun compareTest5() {
        assertEquals(
            BigDecimal("6075.9").divide(BigDecimal("0.5"), 0, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("6075.9").divide(BN.parseString("0.5"), DecimalMode(100, BNMode.CEILING, 0)).toPlainString()
        )
    }

    @Test
    fun compareTest6() {
        assertEquals(
            BigDecimal("0.001294914096").divide(BigDecimal("0.000001"), 0, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("0.001294914096").divide(BN.parseString("0.000001"), DecimalMode(100, BNMode.CEILING, 0)).toPlainString()
        )
    }

    @Test
    fun compareRandom() {
        val modes = RoundingMode.values().filterNot { it == RoundingMode.UNNECESSARY }
        for (i in 0..1_000_000) {
            val rand1 = Random.nextDouble(-12313.0, 1233123.0)
            val rand2 = Random.nextDouble(-12313.0, 1233123.0)
            if (rand2 == 0.0) continue
            val randScale = Random.nextInt(0, 80)
            val randMode = modes.random()
            val randModeBN = randMode.toBNMode()
            assertEquals(
                rand1.toString().toBigDecimal().divide(rand2.toString().toBigDecimal(), randScale, randMode).stripTrailingZeros().toPlainString(),
                BN.parseString(rand1.toString()).divide(BN.parseString(rand2.toString()), DecimalMode(100 + randScale.toLong(), randModeBN, randScale.toLong())).toPlainString()
            ) 
        }
    }

    private fun RoundingMode.toBNMode(): BNMode =
        when (this) {
            RoundingMode.UP -> BNMode.AWAY_FROM_ZERO
            RoundingMode.DOWN -> BNMode.TOWARDS_ZERO
            RoundingMode.CEILING -> BNMode.CEILING
            RoundingMode.FLOOR -> BNMode.FLOOR
            RoundingMode.HALF_UP -> BNMode.ROUND_HALF_AWAY_FROM_ZERO
            RoundingMode.HALF_DOWN -> BNMode.ROUND_HALF_TOWARDS_ZERO
            RoundingMode.HALF_EVEN -> BNMode.ROUND_HALF_CEILING
            RoundingMode.UNNECESSARY -> BNMode.NONE
        }
}

[avently]

Thanks for reporting the issue, unfortunately I don't understand fully what seems to be the problem

The problem that the library mathematically incorrect. I believe that Java's math was reviewed multiple times and since it's used everywhere I suppose that it's correct by default. If results of any other lib does not match with it, the lib doing something wrong.

The BigNum library is a multiplatform library, there is no need to use expect/actual mechanism with it

Right now I've no idea will I use it or not so better to have an expect/actual declaration. I use only a small subset of features, your lib in this field is the same. So I don't need an equal API with Java but I need equal results of the same calls since it's math, it's not a UI:)

Did you setup test class? I used JVM only, it's enough for testing when you have one code for all platforms in the lib.

[ionspin]

Thanks for the test class, now I see what you are referring to, initially I thought you were talking about string formatting, but you found a bug in rounding code!

[ionspin]

Hopefully this pull request fixes the issues you were seeing, the tests are now passing, and a large test for rounding was a great idea, I fixed several other corner cases where rounding wouldn't return correct results by running it. The fix should be available in 0.3.0-SNAPSHOT as soon as the builds finish.

Once again, thanks for report and the replication test cases!

[avently]

Thanks, you fixed those cases! Found another ones:

@Test
    fun compareTest7() {
        assertEquals(
            BigDecimal("100.2").divide(BigDecimal("0.00000001"), 0, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("100.2").divide(BN.parseString("0.00000001"), DecimalMode(100, BNMode.CEILING, 0)).toPlainString()
        )
    }
    @Test
    fun compareTest8() {
        assertEquals(
            BigDecimal("6075.5").divide(BigDecimal("0.5"), 0, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("6075.5").divide(BN.parseString("0.5"), DecimalMode(100, BNMode.CEILING, 0)).toPlainString()
        )
    }

    @Test
    fun compareTest9() {
        assertEquals(
            BigDecimal("0.002").divide(BigDecimal("0.000001"), 0, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("0.002").divide(BN.parseString("0.000001"), DecimalMode(100, BNMode.CEILING, 0)).toPlainString()
        )
    }

    @Test
    fun compareTest10() {
        assertEquals(
            BigDecimal("1").divide(BigDecimal("80"), 15, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("1").divide(BN.parseString("80"), DecimalMode(100, BNMode.CEILING, 15)).toPlainString()
        )
    }

    @Test
    fun compareTest11() {
        assertEquals(
            BigDecimal("77090").divide(BigDecimal("7709"), 15, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("77090").divide(BN.parseString("7709"), DecimalMode(100, BNMode.CEILING, 15)).toPlainString()
        )
    }

    @Test
    fun compareTest12() {
        assertEquals(
            BigDecimal("0.934").divide(BigDecimal("0.934"), 15, RoundingMode.CEILING).stripTrailingZeros().toPlainString(),
            BN.parseString("0.934").divide(BN.parseString("0.934"), DecimalMode(100, BNMode.CEILING, 15)).toPlainString()
        )
    }

[ionspin]

Thanks for those as well! I'll try to solve them over the weekend!

[ionspin]

Actually this was just a small regression, fix is on the way.

[ionspin]

Fix is in the master, should be available under 0.3.0-SNAPSHOT once this build is done https://gitlab.com/ionspin-github-ci/kotlin-multiplatform-bignum-ci/-/pipelines/280415338

Sometimes windows runner gets stuck even if the build is successful, so if that happens I'll restart it tomorrow.

Once again @avently thanks for the report and the test cases!

[avently]

Now all my tests were passed, thank you for such fast fixes!
Where did you learn about math under the hood of the library? I want to learn the lowest level of math that stands behind variable length arithmetic but don't know where to start from. There are many videos in Youtube but maybe you have a short path to the source of such knowledge

[ionspin]

My pleasure!

As far as learning about building the arithmetic library I would say it's hard to find one place, I listed some of the sources I used at the end of the README, like:

Modern Computer Arithmetic
Richard P. Brent and Paul Zimmermann
Version 0.5.9 of 7 October 2010

Hacker`s Delight
Henry S. Warren, Jr.
Second Edition

Art of Computer Programming, Volume 2: Seminumerical Algorithms
Donald E. Knuth
3rd Edition

Refinement of a newton reciprocal algorithm for arbitrary precision numbers
Yiping Cheng, Ze Liu

But also by reading from the libraries like GMP, Java BigInteger etc.

Art of Computer programming I mostly just used to get the division algorithm, and I think that my implementation is suboptimal because it relies on 32 bit arithmetic at one point. A lot of room for improvement there, and in the rest of the library in general.

I think Modern Computer Arithmetic is a great starting point, there you can pick up the notation and general concepts. It all boils down to implementing integers, because they you implement arbitrary decimals on top of that. I really don't consider myself an expert in the area though :)

[ionspin]

Here you can find Modern Computer Arithmetic downloads https://maths-people.anu.edu.au/~brent/pub/pub226.html

[avently]

Awesome, thanks! The book is my starting point of understanding how similar libs work.