0

I wrote a piece of code to generate primes using the sieve of Eratosthenes:

def prime_list(N):
    p = 2
    l = [x for x in range(p,N)]
    new_l = [p]
    while p**2 < N:
        l = [l[x] for x in range(1,len(l)) if not l[x]%p == 0]
        p = l[0]
        new_l.append(p)
    [l.insert(0,x) for x in new_l[len(new_l)-2::-1]]
    return l

Can someone check if this code is correct? I think it is because it's way faster than the code I used to generate primes before:

for x in range(2,N):
    for y in range(2,x):
        if x%y == 0:
            break
    else:
        print(x)
Ol' Reliable
  • 547
  • 1
  • 8
  • 19
  • Duplicate: http://stackoverflow.com/questions/3939660/sieve-of-eratosthenes-finding-primes-python?rq=1 – meawoppl Mar 11 '14 at 23:32
  • ...does it give the right answer? – jonrsharpe Mar 11 '14 at 23:33
  • yes it does give the right answer – Ol' Reliable Mar 11 '14 at 23:33
  • 2
    You won't find any better authority on finding primes in Python than this question and its answers: http://stackoverflow.com/questions/2068372/fastest-way-to-list-all-primes-below-n-in-python – Andrew Clark Mar 11 '14 at 23:37
  • 1
    If your code has division then it is not the Sieve of Eratosthenes. See [The Genuine Sieve of Eratosthenes](http://www.cs.hmc.edu/~oneill/papers/Sieve-JFP.pdf) – jfs Mar 11 '14 at 23:50
  • can't you just run your two versions and compare hteir outputs, say, up to 1000? 2000? re-check yourself with WolframAlpha, query `Prime[1000]` for a 1000-th prime, `PrimePi[1000]` to find how many primes are below 1000. if it doesn't work, post error message here, or the erroneous output; if it does work OK and you want a review of your code then re-post this question on http://codereview.stackexchange.com – Will Ness Mar 13 '14 at 07:23
  • yes, but i know that BOTH of those codes output primes correctly, except the first piece is much more faster than the second one. I just want to know if it's right or not. I wrote another code after @J.F.Sebastian said that if the code has division, that it's no the SOE. – Ol' Reliable Mar 14 '14 at 02:26
  • if the output is right, then the code is correct *by definition*, no? -- you can make your 2nd code as fast the the 1st, with this correction: `for y in range(2, sqrt(x)+1): ...`. if you want your *correct-output-producing* function reviewed, post it on codereview site that I mentioned. -- a proper speed comparison is ***never*** at one size-point; use two or three size points and calculate the [empirical orders of growth](https://en.wikipedia.org/wiki/Analysis_of_algorithms#Empirical_orders_of_growth). – Will Ness Mar 14 '14 at 09:26

1 Answers1

-1

Here is the other code I wrote. I will now check if this one or the other is better.

import math
def primes(N):
    number_list, limit = [x for x in range(2,N+1)], int(math.sqrt(N))
    for n in range(0, limit):
        if number_list[n] != -1:
            for y in range(number_list.index(number_list[n])+1, len(number_list)):
                if number_list[y] != -1:
                    if number_list[y]%number_list[n] == 0:
                        number_list[y] = -1
    number_list = list(set(number_list))
    number_list.remove(-1)
    number_list.sort()
    return number_list

Ok, so this piece of code took 15.90 seconds to compute primes up to 1,000,000. The first one that I posted took only 4.71 seconds to computer primes up to 1,000,000. J.F. Sebastian said that if your code has division, then it's not the SOE but in order to see if each number is a multiple of the remaining numbers, you need to use the modulus operator (which basically is like division), is it not?

Ol' Reliable
  • 547
  • 1
  • 8
  • 19
  • Look at [`sieve_of_eratosthenes(limit)` function](http://stackoverflow.com/a/20782064/4279) to see how to implement it without division (or modulo). To improve time performance, see [Speed up bitstring/bit operations in Python?](http://stackoverflow.com/q/2897297/4279) and [Most efficient code for the first 10000 prime numbers?](http://stackoverflow.com/q/622/4279) in addition to [Fastest way to list all primes below N in python](http://stackoverflow.com/q/2068372/4279) linked by [@F.J](http://stackoverflow.com/questions/22338936?#comment33950452_22338936) – jfs Mar 14 '14 at 08:51
  • a proper speed comparison is ***never*** at one size-point; use two or three size points and calculate the [empirical orders of growth](https://en.wikipedia.org/wiki/Analysis_of_algorithms#Empirical_orders_of_growth). -- It is even possible to define [*infinite* generation of primes](http://stackoverflow.com/questions/2211990/how-to-implement-an-efficient-infinite-generator-of-prime-numbers-in-python/10733621#10733621) without any modulus operations. – Will Ness Mar 14 '14 at 14:33
  • @J.F.Sebastian - ok, thanks for that, It makes sense that he's not using division because he's not using numbers – Ol' Reliable Mar 14 '14 at 17:19
  • @Ol'Reliable: Incorrect. There is no division because the algorithm doesn't require it. If you want to get "numbers" from [the sieve](http://stackoverflow.com/a/20782064/4279): `primes = [i for i, isprime in enumerate(is_prime) if isprime]` or [use a generator](http://stackoverflow.com/a/193605/4279) – jfs Mar 14 '14 at 17:27