-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Various changes to primality testing #2063
Conversation
9205e21
to
fae4d82
Compare
Also remove unused (and unusable, due to hardcoding a broken link) IsBPSWPseudoPrime_VerifyCorrectness
This reflects the current state of the art, see <http://www.trnicely.net/misc/bpsw.html>
While in theory, there might be fewer divisions to compute (as a commented in the removed code suggests), the magnitude of the involved operands also plays a role for efficiency; in a (single) GcdInt operation, the arguments quickly get smaller, so most divisions end up being performed on immediate integers. Some quick experiments comparing the two approaches confirm this, massively: gap> Number([0..2^16],n->ForAny(Primes{[27..168]},p->0=(2^255+n) mod p));; time; 5623 vs. gap> Number([0..2^16], n-> 1<>GcdInt(2^255 + n, > 841284107844892882230924743483896036230303226400884429367479745\ > 182396425076313801080105888842525657179186823477095844441732607\ > 309415612117497325122570590402649274666448191740488756513678929\ > 402959775310209214502833707784648441319210161128261125112776114\ > 119620471154579797706399078932717575475133487349361392344929340\ > 84356041841547537781640044258066541550710400764797315999285813));; time; 242
What specifically do you wish me to do here? |
@Stefan-Kohl I thought you might be familiar with the primality testing code, and thus could possible review the changes in this commit. (UPDATE: err, I meant "PR", not "commit") |
I think Jack Schmidt has written most of the primality testing code. I have so far taken only a quick glance at that code, not more. And already that has been years ago. If you would like to have an expert opinion, I'd suggest you to ask Jack. -- Although I haven't heard from him since a while now, he is still among the members of the GAP Support Group as listed here: http://www.gap-system.org/Contacts/People/supportgroup.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks plausible and is only removing pre-millenium techniques.
@fingolfin we need to add this to release notes for GAP 4.10 at https://github.com/gap-system/gap/wiki/Release-notes-for-GAP-4.10. The current PR title is too generic - perhaps you could suggest a more specific title or edit that wiki page and put a more specific description there? |
See the commit messages for details on the changes in here.