-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
bpo-40028: Add is_prime, factorise, previous_prime and next_prime #19918
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
Conversation
i += 1 | ||
|
||
|
||
def _factorise(n): |
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.
Something like this would help to not exceed maximum recursion depth:
def _factorise(n):
stack = [n]
while stack:
n = stack.pop()
if is_prime(n):
yield n
else:
d = _rho_pollard(n)
stack += [n//d, d]
_factorise
could perhaps then be in-lined, since it seems like factorise
doesn't do too much on its own.
This then passes the test set(factorise(2**10000)) == {2}
.
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.
Thanks!
while q & 1 == 0: | ||
t += 1 | ||
q >>= 1 | ||
u = (n-1)//(2**t) |
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.
Is there a reason to recompute u
here when its value should be the same as q
?
n -= 2 | ||
|
||
|
||
def _rho_pollard(n): |
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.
The following should be exactly the same algorithm, just refactored a little bit, and it runs a little faster for a few tests by my measurements.
def _rho_pollard(n):
# From https://en.wikipedia.org/wiki/Pollard%27s_rho_algorithm
# and https://en.wikipedia.org/wiki/Cycle_detection#Brent.27s_algorithm
tortoise = hare = 2
power = 1
_gcd = gcd
_repeat = itertools.repeat
while True:
# Double the length of the run each time
for _ in _repeat(None, power):
hare = (hare**2 + 1) % n
d = _gcd(hare-tortoise, n)
if d == 1:
continue
elif d == n:
# failure; start another
tortoise = hare = randrange(n)
power = 1
break
else:
return d
else: # no break
tortoise = hare
power *= 2
Changes (feel free to pick and choose as you desire):
- starting at 2 rather than random seems to do good things
- Limit the Python
int
comparisons to one per inner loop- Factor out the inner loop as a
repeat
(a little faster thanrange()
), making c code do the comparisons
- Factor out the inner loop as a
- making _gcd and _repeat local variables helps a little.
Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>
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.
The BPO was closed by @mdickinson
I suggest that your PR into a PyPI package and get feedback from the community.
https://bugs.python.org/issue40028