views:

494

answers:

2

Rather than scraping a Ruby version of this algorithm off the net I wanted to create my own based on its description here. However I cannot figure out two things

def primeSieve(n)
  primes = Array.new

  for i in 0..n-2
   primes[i] = i+2
  end

  index = 0
  while Math.sqrt(primes.last).ceil > primes[index]
    (primes[index] ** 2).step(primes.length - 1, primes[index]) 
      {|x| x % primes[index] == 0 ? primes.delete(x) : ""}
    index += 1
  end

  primes
end
  1. Why it doesn't iterate to the end of the array?
  2. According to the description in the link above the loop should be broken out of when the squareroot of the last element in the array is greater than the current prime - mine does this one before.

I'm fairly sure it has something to do with the delete operation modifying the length of the array. For example my function currently yields 2,3,5,7,9,10 when I enter n=10 which is obviously not correct. Any suggestions on how I can go about alterating this to make it work like it's supposed to?

+1  A: 

The following seems to work. I took out the floating point arithmetic and squared instead of square rooting. I also replaced the deletion loop with a "select" call.

while primes[index]**2 <= primes.last
      prime = primes[index]
      primes = primes.select { |x| x == prime || x%prime != 0 }
      index += 1
end

Edit: I think I figured out how you're trying to do this. The following seems to work, and seems to be more in line with your original approach.

while Math.sqrt(primes.last).ceil >= primes[index]
    (primes[index] * 2).step(primes.last, primes[index]) do
      |x|
      primes.delete(x)
    end
    index += 1
end
Glomek
Much appreciated Glomek! I think the first solution you posted certainly makes more sense, as I previously wasn't aware of the select operation being new to Ruby, I had to look it up. Thanks once again :D
Damian
The second option is horribly slow - the first is about 80x better, but still about 50% slower than the best I have at present.
Mike Woodhouse
I was trying to come up with something as close to the original poster's code as possible, but which worked.
Glomek
A: 

There's a faster implementation at www.scriptol.org:

def sieve_upto(top)
  sieve = []
  for i in 2 .. top
    sieve[i] = i
  end
  for i in 2 .. Math.sqrt(top)
    next unless sieve[i]
    (i*i).step(top, i) do |j|
      sieve[j] = nil
    end
  end
  sieve.compact
end

I think it can be improved on slightly thus:

def better_sieve_upto(n)
  s = (0..n).to_a
  s[0] = s[1] = nil
  s.each do |p|
    next unless p
    break if p * p > n
    (p*p).step(n, p) { |m| s[m] = nil }
  end
  s.compact
end

...largely because of the faster array initialisation, I think, but it's marginal. (I added #compact to both to eliminate the unwanted nils)

Mike Woodhouse