What's the style for immutable set and map in F#

Question

I have just solved problem23 in Project Euler, in which I need a set to store all abundant numbers. F# has a immutable set, I can use Set.empty.Add(i) to create a new set containing number i. But I don't know how to use immutable set to do more complicated things.

For example, in the following code, I need to see if a number 'x' could be written as the sum of two numbers in a set. I resort to a sorted array and array's binary search algorithm to get the job done.

Please also comment on my style of the following program. Thanks!

let problem23 = 
    let factorSum x =
        let mutable sum = 0
        for i=1 to x/2 do
            if x%i=0 then
                sum <- sum + i
        sum
    let isAbundant x = x < (factorSum x)
    let abuns = {1..28123} |> Seq.filter isAbundant |> Seq.toArray
    let inAbuns x = Array.BinarySearch(abuns, x) >= 0
    let sumable x = 
        abuns |> Seq.exists (fun a -> inAbuns (x-a))
    {1..28123} |> Seq.filter (fun x -> not (sumable x)) |> Seq.sum

the updated version:

let problem23b =
    let factorSum x =
        {1..x/2} |> Seq.filter (fun i->x%i=0) |> Seq.sum
    let isAbundant x = x < (factorSum x)
    let abuns = Set( {1..28123} |> Seq.filter isAbundant )
    let inAbuns x = Set.contains x abuns  
    let sumable x = 
        abuns |> Seq.exists (fun a -> inAbuns (x-a))
    {1..28123} |> Seq.filter (fun x -> not (sumable x)) |> Seq.sum

This version runs in about 27 seconds, while the first 23 seconds(I've run several times). So an immutable red-black tree actually does not have much speed down compared to a sorted array with binary search. The total number of elements in the set/array is 6965.

Answer 1

You can easily create a Set from a given sequence of values.

let abuns = Set (seq {1..28123} |> Seq.filter isAbundant)

inAbuns would therefore be rewritten to

let inAbuns x = abuns |> Set.mem x

Seq.exists would be changed to Set.exists

But the array implementation is fine too ...

Note that there is no need to use mutable values in factorSum, apart from the fact that it's incorrect since you compute the number of divisors instead of their sum:

let factorSum x = seq { 1..x/2 } |> Seq.filter (fun i -> x % i = 0) |> Seq.sum

Answer 2

Your style looks fine to me. The different steps in the algorithm are clear, which is the most important part of making something work. This is also the tactic I use for solving Project Euler problems. First make it work, and then make it fast.

As already remarked, replacing Array.BinarySearch by Set.contains makes the code even more readable. I find that in almost all PE solutions I've written, I only use arrays for lookups. I've found that using sequences and lists as data structures is more natural within F#. Once you get used to them, that is.

I don't think using mutability inside a function is necessarily bad. I've optimized problem 155 from almost 3 minutes down to 7 seconds with some aggressive mutability optimizations. In general though, I'd save that as an optimization step and start out writing it using folds/filters etc. In the example case of problem 155, I did start out using immutable function composition, because it made testing and most importantly, understanding, my approach easy.

Picking the wrong algorithm is much more detrimental to a solution than using a somewhat slower immutable approach first. A good algorithm is still fast even if it's slower than the mutable version (couch hello captain obvious! cough).

Edit: let's look at your version

Your problem23b() took 31 seconds on my PC.

Optimization 1: use new algorithm.

//useful optimization: if m divides n, (n/m) divides n also
//you now only have to check m up to sqrt(n)
let factorSum2 n = 
    let rec aux acc m =
        match m with
        | m when m*m = n -> acc + m
        | m when m*m > n -> acc
        | m -> aux (acc + (if n%m=0 then m + n/m else 0)) (m+1)
    aux 1 2

This is still very much in functional style, but using this updated factorSum in your code, the execution time went from 31 seconds to 8 seconds.

Everything's still in immutable style, but let's see what happens when an array lookup is used instead of a set:

Optimization 2: use an array for lookup:

let absums() = 
    //create abundant numbers as an array for (very) fast lookup
    let abnums = [|1..28128|] |> Array.filter (fun n -> factorSum2 n > n)
    //create a second lookup: 
    //a boolean array where arr.[x] = true means x is a sum of two abundant numbers
    let arr = Array.zeroCreate 28124
    for x in abnums do 
        for y in abnums do
            if x+y<=28123 then arr.[x+y] <- true
    arr

let euler023() = 
    absums() //the array lookup
    |> Seq.mapi (fun i isAbsum -> if isAbsum then 0 else i) //mapi: i is the position in the sequence
    |> Seq.sum

//I always write a test once I've solved a problem.
//In this way, I can easily see if changes to the code breaks stuff.
let test() = euler023() = 4179871 

Execution time: 0.22 seconds (!).

This is what I like so much about F#, it still allows you to use mutable constructs to tinker under the hood of your algorithm. But I still only do this after I've made something more elegant work first.

Answer 3

Here is a simple functional solution that is shorter than your original and over 100× faster:

let problem23 =
  let rec isAbundant i t x =
    if i > x/2 then x < t else
      if x % i = 0 then isAbundant (i+1) (t+i) x else
        isAbundant (i+1) t x
  let xs = Array.Parallel.init 28124 (isAbundant 1 0)
  let ys = Array.mapi (fun i b -> if b then Some i else None) xs |> Array.choose id
  let f x a = x-a < 0 || not xs.[x-a]
  Array.init 28124 (fun x -> if Array.forall (f x) ys then x else 0)
  |> Seq.sum

The first trick is to record which numbers are abundant in an array indexed by the number itself rather than using a search structure. The second trick is to notice that all the time is spent generating that array and, therefore, to do it in parallel.