Trying to write an Eratosthenes Sieve in Python. Is this correct and how can I make it faster?

Question

Possible Duplicate:
Fastest way to list all primes below N in python

I have not been doing programming for very long, and I'm just doing this for fun, and I don't know much advanced Python, but... I wrote this, and I wanted to know whether it is actually an Eratosthenes Sieve program, and if it is, how could I make it faster. I don't really want someone to post a program that is a solution, but more tell me how I could adapt mine.

def eratSieve(n):
    all = []
    for a in range(2, n+1):
        all.append(a)               
    for b in all:                       
        for i in range(2,int(round(len(all)/b))):
            while i*b in all:
                all.remove(i*b)
                i+=1
    return all

Thanks for your help.

BTW - It's in Python 2.7

I think you should take a look at [this question](http://stackoverflow.com/questions/2068372/fastest-way-to-list-all-primes-below-n-in-python). :) — Andy Hayden, Dec 14 '12 at 14:05

Gianluca · Accepted Answer · 2012-12-14T14:38:29.913

It does not work right.

The main problem is that you loop on all the value in all and in the while you remove some element from all.

This way some value in all are not considered, so the function does not remove all the non-prime numbers

Try to execute it for n=100 and the result you get is

2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 49, 51, 53, 55, 57, 59, 61, 63, 65, 67, 69, 71, 73, 75, 77, 79, 81, 83, 85, 87, 89, 91, 93, 95, 97, 99

while it should be

2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97 (from http://en.wikipedia.org/wiki/Prime_number)

Also, the range of the second for is wrong, since you consider the lenght of the list, not the current value of b and so you check for multiple of 2 only in the first 50 values, the multiple of 3 in the first 17, 5 in the first 9 and so on. From b = 13 you never enter in the inner for, since int(round(len(all)/b)) = 1 and so you have something like for i in range(2,1)

score 0 · Answer 2 · answered Dec 14 '12 at 14:52

I agree with Gianluca, and I have a possible solution: keep your main array (all) as bools and mark non-primes, but don't remove them. It might also be faster, because you don't change list size.

Minor thing: you can just write all = range(2, n+1) in the beginning if you want to keep it as ints.

John Fink · Answer 3 · 2012-12-14T16:18:19.813

Your method produces incorrect results. The error lies in the for i loop. Here it is, adjusted, and with a test:

known_primes = [
2,3,5,7,11,13,17,19,23,29,
31,37,41,43,47,53,59,61,67,71,
73,79,83,89,97,101,103,107,109,113,
127,131,137,139,149,151,157,163,167,173,
179,181,191,193,197,199,211,223,227,229,
233,239,241,251,257,263,269,271,277,281,
283,293,307,311,313,317,331,337,347,349,
353,359,367,373,379,383,389,397,401,409,
419,421,431,433,439,443,449,457,461,463,
467,479,487,491,499,503,509,521,523,541,
547,557,563,569,571,577,587,593,599,601,
607,613,617,619,631,641,643,647,653,659,
661,673,677,683,691,701,709,719,727,733,
739,743,751,757,761,769,773,787,797,809,
811,821,823,827,829,839,853,857,859,863,
877,881,883,887,907,911,919,929,937,941,
947,953,967,971,977,983,991,997,1009,1013,
1019,1021,1031,1033,1039,1049,1051,1061,1063,1069,
1087,1091,1093,1097,1103,1109,1117,1123,1129,1151,
1153,1163,1171,1181,1187,1193,1201,1213,1217,1223,
1229,1231,1237,1249,1259,1277,1279,1283,1289,1291,
1297,1301,1303,1307,1319,1321,1327,1361,1367,1373,
1381,1399,1409,1423,1427,1429,1433,1439,1447,1451,
1453,1459,1471,1481,1483,1487,1489,1493,1499]
def eratSieve(n):
    all = []
    for a in range(2, n+1):
        all.append(a)               
    for b in all:                       
        for i in all[all.index(b):]:
            while i*b in all:
                all.remove(i*b)
                i+=1
    return all

for N in range(1500):
    for n in eratSieve(N):
        if n not in known_primes:
            print N,n

Are you sure it work ? The output of the function `eratSieve` for `n=100` is `[2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 53, 59, 61, 67, 71, 73,79, 83, 89, 97, 99]` — Gianluca, Dec 14 '12 at 16:05
You're right, it needs a `+1` in that loop. I've edited accordingly. — John Fink, Dec 14 '12 at 16:07

score -1 · Answer 4 · answered Dec 14 '12 at 14:10

def primes(N):
    primes = [x for x in (2, 3, 5, 7, 11, 13) if x < N]
    if N < 17: return primes
    candidators = [x for x in xrange((N - 2) | 1, 15, -2)
                    if x % 3 and x % 5 and x % 7 and x % 11 and x % 13]
    top = int(N ** 0.5)
    while (top + 1) * (top + 1) <= N: top += 1
    while True:
        p = candidators.pop()
        primes.append(p)
        if p > top: break
        candidators = filter(p.__rmod__, candidators)
    candidators.reverse()
    primes.extend(candidators)
    return primes

I think this code would work faster...

Trying to write an Eratosthenes Sieve in Python. Is this correct and how can I make it faster?

4 Answers4