Karatsuba added, 2 bugs are fixed #328

2021-05-11T01:53:21+03:00

zhelenskiy commented

2021-05-11 01:53:21 +03:00

(Migrated from github.com)

Karatsuba was added and increased benchmarks results.
The bug that was caused by abs(Int.MIN_VALUE) == Int.MIN_VALUE was fixed.
The equals function now behaves with instances of other types as all other objects do.

* Karatsuba was added and increased benchmarks results. * The bug that was caused by `abs(Int.MIN_VALUE) == Int.MIN_VALUE` was fixed. * The `equals` function now behaves with instances of other types as all other objects do.

zhelenskiy commented

2021-05-11 01:54:51 +03:00

(Migrated from github.com)

(Slack discussion)

*(Slack discussion)*

breandan commented

2021-05-11 02:45:28 +03:00

(Migrated from github.com)

I recently learned that E.A. Karatsuba has a fast method for evaluating transcendentals, which can be useful for working with certain probability distributions. It might be worth looking into at some point: https://en.wikipedia.org/wiki/FEE_method

zhelenskiy commented

2021-05-11 03:48:19 +03:00

(Migrated from github.com)

@breandan However, the Karatsuba's algorithm that is subject of the PR is fast multiplication.

@breandan However, the Karatsuba's algorithm that is subject of the PR is [fast multiplication](https://en.wikipedia.org/wiki/Karatsuba_algorithm).

altavir (Migrated from github.com) requested changes 2021-05-11 09:00:09 +03:00

altavir (Migrated from github.com) left a comment

The changes in the core are not appropriate at the moment, they should be discussed in a separate issue first. Otherwise looks fine. It would be nice to have your benchmarking results in docs and a separate example in examples.

CommanderTvis (Migrated from github.com) requested changes 2021-05-11 12:04:05 +03:00

zhelenskiy (Migrated from github.com) reviewed 2021-05-11 13:11:08 +03:00

zhelenskiy (Migrated from github.com) reviewed 2021-05-11 13:16:07 +03:00

pklimai (Migrated from github.com) reviewed 2021-05-11 20:11:32 +03:00

zhelenskiy commented

2021-05-12 05:02:08 +03:00

(Migrated from github.com)

I have submitted what you suggested. Also, I fixed bugs, listed in Slack. I decreased the overhead of parsing of BigInt.

zhelenskiy (Migrated from github.com) reviewed 2021-05-12 11:34:13 +03:00

zhelenskiy commented

2021-05-12 11:46:55 +03:00

(Migrated from github.com)

The changes in the core are not appropriate at the moment, they should be discussed in a separate issue first. Otherwise looks fine. It would be nice to have your benchmarking results in docs and a separate example in examples.

I think they should be discussed in Slack because it is already in the PR
Which results? Furthermore, there is still a Fourier -based multiplication method to be implemented first to publish results then.

> The changes in the core are not appropriate at the moment, they should be discussed in a separate issue first. Otherwise looks fine. It would be nice to have your benchmarking results in docs and a separate example in examples. * I think they should be discussed in Slack because it is already in the PR * Which results? Furthermore, there is still a Fourier -based multiplication method to be implemented first to publish results then.

zhelenskiy commented

2021-05-12 14:17:38 +03:00

(Migrated from github.com)

I found that the addition of large integers is slower with KMath BigInt than with JVM BigInteger: I added benchmarks for that.