15

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?

Damian
  • 663
  • 1
  • 9
  • 12
  • how about some whitespace so i can brainparse your code? – Dustin Getz Oct 27 '08 at 23:22
  • 1
    Well, this little experience has turned me off Ruby for a while. It appears to have all the expressive capability of Perl with the atrocious readability of Perl, but at least I already understand Perl. – paxdiablo Oct 27 '08 at 23:33
  • 8
    You probably shouldn't judge Ruby from this example. I think Damian is new to Ruby, and this isn't the normal way to implement the Sieve of Eratosthenes. – Andru Luvisi Oct 28 '08 at 00:00

5 Answers5

17

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
  • 51,832
  • 12
  • 88
  • 127
5

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
Andru Luvisi
  • 24,367
  • 6
  • 53
  • 66
  • 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 Oct 28 '08 at 00:06
  • 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 Jan 11 '09 at 12:42
  • I was trying to come up with something as close to the original poster's code as possible, but which worked. – Andru Luvisi Jan 12 '09 at 14:40
  • 1
    The second snippet is crap. Call "delete" at your own peril. You try and run that for N = 10 million – nes1983 Mar 05 '12 at 09:13
  • @nes1983 I'm not familiar with Ruby, does `a.delete(x)` takes time linear in length of `a` perhaps? That would indeed break the whole premise of the sieve, which is not having to compare *values* but rather *use addresses directly*, as stand-in's for values. The first snippet is no good either, testing *each* element in array for divisibility - instead of hopping over it with a step of `p` (or `2p` even - for *odds* ;) ). – Will Ness Mar 10 '12 at 17:42
  • 1
    we're not supposed to actually *remove* anything from the sequence, just mark on it -- the WP article at the time of your answer (and for years yet after that) was confused and misleading - *wrong*. – Will Ness Mar 10 '12 at 17:48
  • Will Ness is correct. Neither of these two solutions implements the sieve. They are simply variations of trial division, which has a worse time complexity. – Jørgen Fogh Dec 11 '14 at 13:42
3

This is a pretty straightforward implementation of the Wikipedia article pseudocode, using a bit array.

#!/usr/bin/env ruby -w

require 'rubygems'
require 'bitarray'

def eratosthenes(n)

   a = BitArray.new(n+1)

   (4..n).step(2) { |i|
      a[i] = 1
   }

   (3..(Math.sqrt(n))).each { |i|
       if(a[i] == 0)
           ((i*i)..n).step(2*i) { |j|
               a[j] = 1
           }
       end
   }
   a
 end

def primes(n)
    primes = Array.new
     eratosthenes(n).each_with_index { |isPrime, idx|
        primes << idx if isPrime == 0
     }
     primes[2..-1]
end
nes1983
  • 15,209
  • 4
  • 44
  • 64
  • 1
    if you'll look into the article text itself you'll see that you actually can stop at `sqrt(n)`, start from `(i*i)` and use step of `2*i` for all primes except 2. – Will Ness Mar 10 '12 at 17:37
  • perhaps you should also compact the array and translate the *true* addresses into straight up numbers somehow? – Will Ness Mar 10 '12 at 17:58
  • @WillNess What you want is a bit array. I don't think Ruby has native support for them, though. https://github.com/ingramj/bitarray – nes1983 Mar 11 '12 at 17:59
  • no, I mean you return your `a` as array of true's and false's; should't it be converted into a list/array of numbers in the end? – Will Ness Mar 11 '12 at 18:04
  • @WillNess `primes = Array.new; bitarray.each_with_index {|isPrime, index| primes << index if isPrime}` – nes1983 Mar 11 '12 at 19:38
1

This is a reference for those who are interested. The code is from this site.

This code uses Sieve of Eratosthenes as well.

n = 1000000
ns = (n**0.5).to_i + 1
is_prime = [false, false] + [true]*(n-1)
2.upto(ns) do |i|
  next if !is_prime[i]
  (i*i).step(n, i) do |j|
    is_prime[j] = false
  end
end

count = 0
list = (0..n).map do |i|
  count += 1 if is_prime[i]
  count
end

while gets
  puts list[$_.to_i]
end

And here is another one.

def eratosthenes(n)
  nums = [nil, nil, *2..n]
  (2..Math.sqrt(n)).each do |i|
    (i**2..n).step(i){|m| nums[m] = nil}  if nums[i]
  end
  nums.compact
end

p eratosthenes(100)
shin
  • 31,901
  • 69
  • 184
  • 271
0

or

x = []
Prime.each(123) do |p|
  x << p
end

There may be a way to use inject here but the inception thing hurts my head today.

pjammer
  • 9,489
  • 5
  • 46
  • 56
  • The original question is about making sieves, list_of_primes = Prime.each(123).inject([], :<<) would work if you want to use inject/reduce, but using just saying list_of_primes = Prime.each(123) might be smarter in a lot of situations. – user2251284 Nov 20 '14 at 03:01