2

I need to express the sequence of prime numbers. (struggling with ex 3 in project Euler).

I have happened to this recursive definition:

is_not_dividable_by :: (Integral a) => a -> a -> Bool
is_not_dividable_by x y = x `rem` y /= 0

accumulate_and :: (Integral a) => [a] -> (a -> Bool) -> Bool
accumulate_and (x:xs) (f) = (accumulate_and xs (f)) && f(x)
accumulate_and [] f = True

integers = [2,3..]

prime_sequence = [n | n <- integers, is_prime n]
                where is_prime n = accumulate_and 
                                        (takeWhile (<n) (prime_sequence)) 
                                        ( n `is_not_dividable_by`)

result = take 20 prime_sequence
str_result = show result
main = putStrLn str_result

Though it compiles well, but when executed, it falls into a loop, and just returns <<loop>>

My problem is that I think that I can freely express recursive definitions in Haskell. But obviously this definition does not fit with the language at all.

However, when I mentally try to solve the prime_sequence, I think I succeed and grow the sequence, but of course with imperative programming apriori.

What is plain wrong in my recursive definition, that makes this code not work in Haskell ?

Stephane Rolland
  • 38,876
  • 35
  • 121
  • 169
  • I can still see obvious bugs in your code that makes it impossible that it compiles. I strongly recommend using actual copy and paste rather than trying to retype it, and get it wrong. (Although for SO you need to make sure to indent 4 extra spaces first). – Ørjan Johansen Sep 13 '14 at 02:50
  • 2
    (P.S. You can use the "{}" button in the editor to automatically indent your selection by 4 spaces.) – Rufflewind Sep 13 '14 at 02:51
  • I suspect the infinite loop has something to do with the fact that your `is_prime` requires elements from `prime_sequence` that aren't yet computed. – Rufflewind Sep 13 '14 at 02:56
  • Don't worry about the indentation part (it was just a general suggestion). The formatting is fine. – Rufflewind Sep 13 '14 at 02:59
  • @StephaneRolland I think that was just a comment to my wrong claim you needed to indent it before pasting. – Ørjan Johansen Sep 13 '14 at 02:59

2 Answers2

3

The reason it's an infinite loop is because of this line:

prime_sequence =
  [n | n <- integers, is_prime n]
  where is_prime n = accumulate_and (takeWhile (< n) prime_sequence)
                                    (n `is_not_dividable_by`)

In order to compute is_prime n, it needs to take all the prime numbers less than n. However, in order for takeWhile to know when to stop taking it needs need to also check for n, which hasn't been computed yet.

(In a hand-wavy manner, it means your prime_sequence is too lazy so it ends up biting its own tail and becoming an infinite loop.)

Here's how you can generate an infinite list of prime numbers without running into an infinite loop:

-- | An infinite list of prime numbers in ascending order.
prime_sequence :: [Integer]
prime_sequence = find [] integers
  where find :: [Integer] -> [Integer] -> [Integer]
        find primes [] = []
        find primes (n : remaining)
          | is_prime   = n : find (n : primes) remaining
          | otherwise  = find primes remaining
          where is_prime = accumulate_and primes (n `is_not_dividable_by`)

The important function here is find, which takes an existing list of primes and a list of remaining integers and produces the next remaining integer that is prime, then delays the remaining computation until later by capturing it with (:).

Rufflewind
  • 8,545
  • 2
  • 35
  • 55
  • *"it needs to take all the prime numbers from zero to (but not including) n"* how does it know that it's *"from 0"*? It doesn't. :) – Will Ness Sep 13 '14 at 06:25
  • `accumulate_and [a,b,c] f = (accumulate_and [b,c]) && f a` is different from `and (map f [a,b,c]) = f a && (and (map f [b,c]))`. – Will Ness Sep 13 '14 at 08:43
  • Even simpler, `and (map f xs) = all f xs`. – Ørjan Johansen Sep 13 '14 at 16:22
  • @ØrjanJohansen ah yes, :) but it's not the same as the OP's function. Although that is most probably an oversight on their part. – Will Ness Sep 13 '14 at 18:34
  • @WillNess Not the same, but more concise. – Stephane Rolland Sep 13 '14 at 20:04
  • 1
    @WillNess You almost certainly want to do the check in the standard `and/all` order, since that is tail recursive and shortcutting. – Ørjan Johansen Sep 14 '14 at 00:33
  • @ØrjanJohansen yes. though you meant *guarded* recursive, and short-circuiting *in proper order* (OP's function is short-circuiting on elements too, from end, after forcing all the spine through), so the trial divisions are tested from smaller primes to bigger ones (as it should be), unlike the OP's variant that works from bigger primes towards smaller ones, which should slow it down considerably, because more numbers have smaller prime factors than the bigger ones. In code like in this answer though, with the separate *finite list of primes in reverse*, OP's version *is* better! :) – Will Ness Sep 14 '14 at 06:30
  • @StephaneRolland it's not the same *operationally*: the `and`/`all` versions will call `f a` first, your code will call `f c` first. Sometimes (not here) `c` might still not be there, whereas `a` is already present, and `f a` would return `False` stopping the further calculations. In such situation that would mean a difference between operating correctly and causing an error. – Will Ness Sep 14 '14 at 06:34
  • @WillNess Hm, on checking what the term means, I am pretty sure I do *not* mean guarded recursive - that applies to building structures with lazy constructors. `and/all` generate a simple `Bool`. But if you check with equational reasoning how lazy evaluation expands the term `and (x:xs)`, you will see that if `and xs` ever gets evaluated, it will do so by becoming the whole top level expression - which in my understanding is the essence of an optimized tail call in Haskell. – Ørjan Johansen Sep 14 '14 at 12:58
  • @ØrjanJohansen well, `&&` is *semi*-strict so it could be seen as guarding the recursion on its right. the RHS of `and (x:xs) = x && and xs` doesn't *look* tail -- `x` should be evaluated first, and it certainly won't be in tail context. So no, it's not tail recursion, or else `f n = f (n-1) && f (n-2)` would be too. – Will Ness Sep 14 '14 at 16:26
3

The culprit is this definition:

prime_sequence = [n | n <- [2,3..], is_prime n]  where 
  is_prime n = accumulate_and 
                 (takeWhile (< n) (prime_sequence)) 
                 ( n `is_not_dividable_by`)

Trying to find the head element of prime_sequence (the first of the 20 to be printed by your main) leads to takeWhile needing to examine prime_sequence's head element. Which leads to a takeWhile call needing to examine prime_sequence's head element. And so it goes, again and again.

That's the black hole, right away. takeWhile can't even start walking along its input, because nothing's there yet.

This is fixed easily enough by priming the sequence:

prime_sequence = 2 : [n | n <- [3,4..], is_prime n]  where 
  is_prime n = accumulate_and 
                 (takeWhile (< n) (prime_sequence)) 
                 ( n `is_not_dividable_by`)

Now it gets to work, and hits the second problem, described in Rufflewind's answer: takeWhile can't stop walking along its input. The simplest fix is to stop at n/2. But it is much better to stop at the sqrt:

prime_sequence = 2 : [n | n <- [3,4..], is_prime n]  where 
  is_prime n = accumulate_and 
                 (takeWhile ((<= n).(^ 2)) (prime_sequence)) 
                 ( n `is_not_dividable_by`)

Now it should work.

Community
  • 1
  • 1
Will Ness
  • 70,110
  • 9
  • 98
  • 181