Skip to content

Conversation

lomereiter
Copy link
Contributor

Multiplication by Karatsuba or Toom-Cook method is not effective when the two numbers are of different order of magnitude (for example, 167483-digit number times 1263-digit one). It's better to slice the larger number into pieces which size is roughly equal to the smaller number size. This algorithm was implemented in Ruby, and it works much faster.

@ghost ghost assigned sjaeckel Jun 1, 2011
@sjaeckel sjaeckel closed this Jan 30, 2014
@sjaeckel
Copy link
Member

Rebased and merged to develop

@sjaeckel
Copy link
Member

Well I just found out that your code works only for 64bit architectures resp. DIGIT_BIT==60!

I will try to have a look at it, but please do that as well

To reproduce on x64:
build like that: CFLAGS="-DMP_32BIT" make test
run like that: ./test
paste in the following data:

lcm
2xxBudbK71ytl0+bHdFqM4nh6WAFRcWlIZpVWZ6AXt
DZwAHcx79Hx5kdYLIvc6zy7GzRjnSgGayApgshEVl+cUIZ0D4LKi5izah5i+SnB010XxTNhVcYkCHCO/v8NfWj0PZ+5kk/W+8AHP7jUhtjkqMX3lEQiQXY/WnTHchrSsYESNzxj
dqTZpkQ546pZYK5dU9wq6T8uVCbi5lejX6M1UMClz9kBnlpcPQ5+OfoPDw5UxoilL/F+KNQB9KWWxhDKF8IcKEOMpBHMKMoSnRj+0oAdneCXZ/H4vVXrXglSbo81/4+z50BbMAO6DBrOqrzXZ12B+sEfaZFBX+UXUDdOKLvJI3le52Wh

also tested with CFLAGS="-DMP_32BIT -g" make test IGNORE_SPEED=1

@sjaeckel sjaeckel reopened this Oct 14, 2014
@sjaeckel
Copy link
Member

And TBH, I think you should clarify possible licensing issues with the ruby team!
that isn't a re-implementation of their algorithm, but a copy of their implementation, which could create some problems I think...

@czurnieden
Copy link
Contributor

And TBH, I think you should clarify possible licensing issues with the ruby team!
Oh, he isn't the original author from Ruy?
IANAL but not a real problem, see https://www.ruby-lang.org/en/about/license.txt

  1. You may modify your copy of the software in any way, provided that
    you do at least ONE of the following:

    a) place your modifications in the Public Domain or otherwise
    make them Freely Available, such as by posting said
    modifications to Usenet or an equivalent medium, or by allowing
    the author to include your modifications in the software.

Libtommath is not Public Domain anymore but still counts as "Freely Available", I think.

For the technical side: I implemented a balancing algorithm myself in a recursive way but on copies to keep it simple (could be done with some pointer juggling, too). I also found out, empirically only, that it makes not much sense if the difference is less than about one percent which is not very much. The second difference is that I did it at limb resolution. The much finer resolution of lomereiter seems to be not only not necessary (see the "one percent" above) but is probably the reason for the test failure, too.

CZ

@sjaeckel
Copy link
Member

Okay true - ruby is also public domain... it seems I can't read anymore...

Well but regarding the technical side...

The much finer resolution of lomereiter seems to be not only not necessary (see the "one percent" above) but is probably the reason for the test failure, too.

Sorry for that question, but what does that mean?

@sjaeckel
Copy link
Member

And the bug also occurs on 64bit architectures resp. DIGIT_BIT==60 but it has not been triggered...
I modified mp_mul() to call mp_balance_mul() and that fails as well.

@czurnieden
Copy link
Contributor

It also fails on at least one 32-bit machine: mine.

Sorry for that question, but what does that mean?

I just tried to be polite and failed English grammar, too, sorry.

He tried to save memory, a commendable approach but he implemented it in not exactly the way he intended to do it. I didn't even looked at it but a difference of a mere 75 decimal digits in calculating a 318 decimal digits long result is a strong hint for off-by-one errors and similar.

So, please, lomereiter, take a look at your code, it should be easily repaired. It would be sad if you don't, balancing is a good idea!

CZ

@sjaeckel
Copy link
Member

I've reverted your commit and created a new branch fix/balance_mul where fixes should be applied to.

@sjaeckel sjaeckel removed their assignment Jan 8, 2019
@minad
Copy link
Member

minad commented Feb 10, 2019

@czurnieden Any comments concerning the status of this PR? Is there hope to get this working and is it worth it?

@czurnieden
Copy link
Contributor

@minad

Is there hope to get this working and is it worth it?

There's always hope ;-)

But, no, not really. I already have one ready and tested and am just waiting until the new version gets out to start a new one with balanced multiplication, fast division, FFT/NTT and more.
Oh, and yes, it is worth it. Benchmarking shows significant results starting in the range KARA_CUTOFF * (2*KARA_CUTOFF). But the price is memory overhead, so it should be able to be switched on/off.

@minad
Copy link
Member

minad commented Feb 11, 2019

@czurnieden I guess it is fine to close this one now. Did I understand correctly that you have a patch locally which could be resubmitted at some point?

@minad minad closed this Feb 11, 2019
czurnieden added a commit to czurnieden/libtommath that referenced this pull request May 29, 2023
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

Successfully merging this pull request may close these issues.

4 participants