Sieve of Eratosthenes - Implementation returning some non-prime values?

Question

I implemented the Sieve of Eratosthenes in Java, from pseudocode:

public static void sieveofEratosthenes(int n) {
    boolean numArray[];

    numArray = new boolean[n];
    for(int i = 0; i < n; i++)
        numArray[i] = true;

    int a = 0;

    for(int i = 2; i < Math.sqrt((double)n); i++) {
        if(numArray[i])  {
            for(int j = (int)Math.pow(i, 2); j < n; a++) {
                numArray[j] = false;
                j += (a * i);
            }
        }
    }

    for(int i = 2; i < n; i++) {
        if(numArray[i])
            System.out.println(i);
    }
}

The output it gives me, when i is 15:

Why are some of these values incorrect? I believe my error is in how I define and use the bool array. Thanks!

What do you think your inner loop (with index `j`) is doing? — Kon, Aug 22 '14 at 04:47

score 4 · Accepted Answer · answered Aug 22 '14 at 04:50

4

        for(int j = (int)Math.pow(i, 2); j < n; a++) {
            numArray[j] = false;
            j += (a * i);
        }

should read

        for(int j = (int)Math.pow(i, 2); j < n; j+=i) {
            numArray[j] = false;
        }

answered Aug 22 '14 at 04:50

BevynQ

8,089
4
25
37

score 2 · Answer 2 · answered Aug 22 '14 at 04:57

How SoE works is that it takes each number and "deletes" all numbers following it that are divisible by it. So basically each number x + k*x where k > 0. This can be done by simply adding x to the initial x^2 and then adding iteratively x to it. Here:

for(int j = (int)Math.pow(i, 2); j < n; a++) {
    numArray[j] = false;
    j += (a * i);
}

You are not adding x but a*x, so you will skip some numbers as a is being incremented (so you will remove 4,6,10,16 etc, see the pattern? it adds 2,4,6 etc to the initial value) so you should stick with:

for(int j = (int)Math.pow(i, 2); j < n; j+=i) {
    numArray[j] = false;
}

score 0 · Answer 3 · answered Aug 22 '14 at 05:12

0

The problem is at line

 j += (a * i);

In loop, this statement gradually multiplies the j by a*i and add it with j.So replace above line with,

 j = (a * i);

It will work. And yes,initialize

a=2

because we don't want numarray[0] or numarray[1] to initialize or use. Do comment if any query. Thanks

answered Aug 22 '14 at 05:12

Raj Kantaria

326
3
12

Also, `a` must be reinitialized inside the `for (int i...)` loop. Try and see what happens if you forget to do this. – ajb Aug 22 '14 at 05:14
yes, it would initialized by `2` after `for(int j...)` and inside `for(int i....)` loop. – Raj Kantaria Aug 22 '14 at 05:22

andand · Answer 4 · 2014-11-19T19:58:22.033

This doesn't directly address your question, but since it's already been answered, I don't see any point in repeating it. Looking at your code, though, I suggest using integer multiplication instead of Math.pow and Math.sqrt to get slightly better performance such as:

for(int i = 2; i*i < n; i++) {
    if(numArray[i])  {
        for(int j = i*i; j < n; j += i) {
            numArray[j] = false;
        }
    }
}

Admittedly these calls will only be made once per iteration of the outer loop, so the improvement may not be very dramatic. But, calling Math.pow and Math.sqrt are likely to be much more compute intensive than a single integer multiplication. Also, if Java performs sufficiently sophisticated optimization, i*i may only get computed once and used in both places saving even more compute cycles. There's also no risk of integer overrunning in this case since i*i is bounded above by the integer n.

Sieve of Eratosthenes - Implementation returning some non-prime values?

4 Answers4

Linked