4

I'm implementing the Sieve of Eratosthenes in Python. It returns composite numbers near the end of the search range:

def primes_Ero(n=1000):
    primes = []
    a = [True]*(n+1)
    a[0] = a[1] = False
    for (i,isprime) in enumerate(a):
        if isprime:
            for n in range(i*i,n+1, i):          
                a[n] = False
            primes.append(i)
    return primes

When using larger numbers, n, I end up with composite numbers. I made a check to see which numbers are composite (compared to a brute force method),

Given n, what numbers are composite:

n= 100; []
n= 500; [493, 497]
n= 1000; [961, 989]
n= 10000; [9701, 9727, 9797, 9853, 9869, 9917, 9943, 9953, 9983, 9991, 9997]

What am I doing wrong?

Sebastian Wozny
  • 16,943
  • 7
  • 52
  • 69
LascieL
  • 47
  • 6

1 Answers1

4

The problem is this line:

 for n in range(i*i, n+1, i):

Initially n is set to the parameter value (default=1000) but after the for loop executes for the first time n will hold i < n < i + n. The second time the for loop is executed is faulty.

You should rename one of the ns that you're using. Consider giving it a proper name like sieve_size which is more descriptive of what it actually does.

One thing I would like to point out is, that while your code is clever you are modifying the list you're iterating over. This is generally considered bad practice.

Sebastian Wozny
  • 16,943
  • 7
  • 52
  • 69
  • 1
    Thank you for this! I was putting together a few different ideas and forgot to rename the parts to something descriptive. This helped a lot. – LascieL Feb 24 '17 at 00:38
  • A comment on your last paragraph: There's no problem with modifying a value of a list by index while iterating over the list. It's only a problem if you're inserting or removing values from the middle of the list (since that changes the indexes of all of the later values). – Blckknght Feb 24 '17 at 18:55