Why is this prime-detection program so slow?

Question

I am trying to make a list of primes less than n, with this code

from math import sqrt
primes=[]
def primes_n(n):
    for i in range(2,n+1):
        for k in range (2, int(sqrt(i))+1):
            if k in primes:
                if i%k==0:
                    break

        else:
            primes.append(i)

    return primes

primes(10000)

But it is taking more than 5 seconds, is it the program or just my computer. I spilled a drink on it a while ago, and it is not as fast anymore, so it might be the computer. If it is the program what should i do to speed it up.

My thought is that the break statement is only breaking out of the for k in range... loop, but it should be breaking the for i in range... loop.

You should take a look at [Fastest way to list all primes below N](http://stackoverflow.com/questions/2068372/fastest-way-to-list-all-primes-below-n) and [Sieve of Eratosthenes](https://en.wikipedia.org/wiki/Sieve_of_Eratosthenes). — Vincent, Jun 24 '15 at 13:35

score 1 · Accepted Answer · edited May 23 '17 at 11:58

Two simple improvements would be to:

Loop over odd numbers since even numbers are not primes, except for 2.
To test a number for primality, loop directly over the primes you found so far.

Runs in 150 ms for primes_n(10**5):

from math import sqrt

def primes_n(n):
    primes = [2]
    for i in range(3, n+1, 2):
        for p in primes:
            if p > int(sqrt(i)):
                primes.append(i)
                break
            if i % k == 0:
                break
    return primes

If you want to be faster than that, you'll have to implement a sieve of Eratosthenes.

See some of the fastest implementations here: Fastest way to list all primes below N.

Ah, yours has even more optimizations than mine. Although, yours could be sped up even further by calculating the square root of `i` before going into the inner loop. That way, you only have to calculate that root once per `i`. — Jacob Zimmerman, Jun 24 '15 at 17:34

Jacob Zimmerman · Answer 2 · 2015-06-24T17:48:24.417

It's slow because there are 999900 loops run (minus the ones that are skipped due to the break, but also not counting the ones done from k in primes).

A couple things you can do to optimize:

Use a set instead of a list to store the primes (k in primes will end up being much faster). Then return sorted(primes) if you need them in order. This is made pointless by the next optimization, though.
reduce the number of iterations in the inner loop by:
1. add 2 to the primes collection initially
2. only iterate through the primes collection, since any number that is larger than a known prime is either a prime itself or has prime factors that are less than itself. It makes no sense to iterate through a bunch of numbers that are known to not be primes then. You clearly realize this, since you only bother to check divisibility by primes.
3. To optimize this further, you can make sure you only iterate through the primes that are less than the square root, but this is practically a micro optimization in comparison to the rest. Edit: I tried this, and apparently calculating the square root takes longer than iterating through the primes list.

My Runtimes:

Your code: runtime of 77ms
Using set instead of list: 37 ms; 48% of the time needed, more than twice as fast
Using primes instead of range to square root: 18ms; 38% of the time from using a set, more than 4x faster than original
Same as previous, but limiting the primes to just those under i's square root: 27ms; still 2.9x faster than original, but slower and more complicated than the previous

Note: I was using pypy, so my run time of 77ms vs your 5-ish seconds is partially due to that. But when I ran it on my regular python 2.7 interpreter, I got 980ms (.98 seconds). So it seems that you ALSO have a slow computer, as well as a poorly optimized algorithm.

So, here's my final code for you:

def primes_n(n):
    primes = [2]
    for i in range(3, n+1):
        for prime in primes:
            if i % prime == 0:
                break
        else:
            primes.append(i)
    return primes

UPDATE: I stopped writing mine in the middle to break for lunch, so I didn't get to see the other guy's additional optimizations.

I had done a poor job of using the square root bit, so using it ISN'T actually slower (poorly implementing it is). So, after applying his optimizations, plus the little one I mentioned in the comment on his answer, I get a run time of 3ms, which is 6x faster than my previous best. Here's that code:

def primes3_2(n):
    primes = [2]
    for i in range(3, n+1, 2):
        root_of_i = sqrt(i)
        for prime in primes:
            if prime <= root_of_i:
                primes.append(i)
                break
            if i % prime == 0:
                break
    return primes

Why is this prime-detection program so slow?

2 Answers2