Prime factorization: doesn't work with large numbers?

Question

I'm trying to find the largest prime factor for a number. The code runs correctly on IDLE when used with smaller numbers, but doesn't seem to print anything to the screen at all when I assign a larger number like 600851475143 to n. Why?

def isPrime(n):
    isPrime = True
    for i in range(2,n-1):
        if n % i == 0:
            isPrime =  False
        return isPrime

largest = 0
n = 600851475143
for i in range(2,n-1):
    if isPrime(i) and n % i == 0:
        largest = i
        n = n / i
        continue

print("The largest prime factor is", largest)

I'm running Python 3.3, by the way.

==============================================================================

Thanks everyone!

I fixed my original code as follows:

def isPrime(n):
    for i in range(2,n-1):
        if n % i == 0:
            return False
    return True

largest = 0
n = 600851475143
for i in range(2,n-1):
    if isPrime(i) and n % i == 0:
        largest = i
        if i == n:
            break
        n = n / i

print("The largest prime factor is", largest)

Like nakedfanatic said, their code runs faster, and I edited it slightly:

largest = 0
n = 600851475143
i = 2
while True:
    if n % i == 0:
        largest = i
        if n == i:
            # finished
            break
        n = n / i
    else:
        i += 1

print("The largest prime factor is", largest)

Answer 1

Another aspect of your code which may be slowing it down is the second half of your code

largest = 0
n = 600851475143
for i in range(2,n-1):
    if isPrime(i) and n % i == 0:
        largest = i
        n = n / i
        continue

Specifically the statement

if isPrime(i) and n % i == 0: 

Per the documentation, the second condition is only evaluated if the first one is True. In your case it would make more sense to reverse the conditions so that computationally les expensive division is performed always and the more expensive isPrime() is only called for the actual factors

largest = 0
n = 600851475143
for i in range(2,n-1):
    if n % i == 0 and isPrime(i):
        largest = i
        n = n / i
        if n == 1: 
            break

Answer 2

More difficult problems may require a different algorithm.

Check this question out: Fastest way to list all primes below N

Your code looks okay, but could take a long time for a large n. Leveraging math can enable you to do this problem orders of magnitude faster.

On that link, I recommend rwh_primes1 for a pure python solution, and primesfrom3to as one that uses numpy. Both of those implementations are fairly short, relatively clear, and do basically the same thing. Those code snippets are written in Python 2, so a translation might look like this:

def rwh_primes1(n):
    sieve = [True] * (n//2)
    for i in range(3, int(n**0.5)+1,2):
        if sieve[i//2]:
            sieve[i*i//2::i] = [False] * ((n-i*i-1)//(2*i)+1)
    return [2] + [2*i+1 for i in range(1,n//2) if sieve[i]]

Answer 3

There are several areas of optimization:

• all factorization only needs to got up to sqrt(n) (inclusive)

• convert isPrime() to a table lookup

  Initialize a lookup table using n, then you compute all primes < sqrt(n) only once and loop through them.
  As comments pointed out, this takes up large memory space. We can use bit flag to cut the memory requirement to 1/8, and we can cut it by a further half if we skip all the even numbers (then have to test if n is even separately). But that may still be daunting for LARGE n.

• (if using current code) return early in isPrime() (by @justhalf)

• loop backwards (from sqrt(n) to 2) when looking up the largest factor

• return early if the quotient is 1 after dividing by a factor (by @justhalf)

• This post (suggested by @prashant) contains more complicated algorithm (making my suggestion very naive ><):

  Fastest way to list all primes below N

• ... (edits are welcome)

Answer 4

Your code is running in O(n²) time, which means it will quickly become unreasonably slow as the size of n increases. That is why your algorithm works for small values, but hangs for large values.

This code does the same thing in O(n) time without doing any prime checking at all, and returns a result instantly:

prime_factors = []
n = 600851475143
i = 2
while True:
    if n % i == 0:
        prime_factors.append(i)
        if n == i:
            # finished
            break
        n = n / i
    else:
        i += 1

print("The largest prime factor is", prime_factors[-1])

Answer 5

It's because you keep trying even if n is already 1.

This code will help you to see the problem:

def isPrime(n):
    for i in xrange(2,n-1):
        if n % i == 0:
            return False
    return True

largest = 0
n = 600851475143
for i in xrange(2,n-1):
    print 'Checking whether %d divides %d' % (i,n)
    if isPrime(i) and n % i == 0:
        largest = i
        n = n / i
        continue

print("The largest prime factor is", largest)

which will produce:

...
Checking whether 6857 divides 6857
Checking whether 6858 divides 1
Checking whether 6859 divides 1
Checking whether 6860 divides 1
Checking whether 6861 divides 1
Checking whether 6862 divides 1
Checking whether 6863 divides 1
Checking whether 6864 divides 1
Checking whether 6865 divides 1
Checking whether 6866 divides 1
Checking whether 6867 divides 1
Checking whether 6868 divides 1
Checking whether 6869 divides 1
Checking whether 6870 divides 1
Checking whether 6871 divides 1
Checking whether 6872 divides 1
Checking whether 6873 divides 1
...

You should break the loop when n becomes 1, so that it won't do unnecessary checking

n = n / i
if n==1:
    break
continue

And anyway, your code might be improved by a lot, haha, see others' suggestions.

Answer 6

isPrime = True
for i in range(2,n-1):
    if n % i == 0:
        isPrime =  False
    return isPrime

This loop always exits the first iteration due to the unconditional return. Try:

for i in range(2,n-1):
    if n % i == 0:
        return False

return True

Also the upper bound n-1 can be reduced to sqrt(n)+1.

Answer 7

Most likely, your code isn't terminating with large n, simply because it takes so long to run through the loop.