Skip to content

Conversation

@sjaeckel
Copy link
Member

@sjaeckel sjaeckel commented May 18, 2019

While looking at the coverage results in #268 I think I found an inconsistency... nothing bad but it still stood out while looking at the code...

mp_prime_rabin_miller_trials:38 leads to 2 MR trials for numbers > 4096 bits

@czurnieden is that change fine that not only 4096bit sized numbers only have to do 1 MR but also numbers >4096bits?

Edit: changed to do always at least 2 Rounds

@sjaeckel sjaeckel mentioned this pull request May 18, 2019
@czurnieden
Copy link
Contributor

czurnieden commented May 18, 2019

is that change fine that not only 4096bit sized numbers only have to do 1 MR but also numbers >4096bits?

Yes.

But now that I think about it: that is the theoretical value. Carefully calculated but still only a theoretical value.
Could we set the minimum to (2) two to sooth my conscience?

@sjaeckel sjaeckel force-pushed the fix-miller-rabin-trials branch from 39500c6 to 888c2d7 Compare May 19, 2019 07:58
@sjaeckel sjaeckel changed the title do only 1 MR round for numbers >4096bits do 2 MR rounds for numbers >=2048bits May 19, 2019
Copy link
Contributor

@czurnieden czurnieden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@sjaeckel sjaeckel force-pushed the fix-miller-rabin-trials branch from 888c2d7 to 51cda5b Compare May 21, 2019 07:48
@sjaeckel sjaeckel merged commit e11f70f into develop May 21, 2019
@sjaeckel sjaeckel deleted the fix-miller-rabin-trials branch May 21, 2019 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants