Addem
Addem

Reputation: 3929

OCaml for-loop seems to be re-executing more than expected

I have the following function

let update s = 
  for i=0 to ((Array.length s) - 2) do 
    for j=0 to (Array.length (s.(i))) - 2 do 
      (s.(i)).(j) <- (s.(i).(j)) + 1;
    done;
  done

and it seems to increase the relevant coordinates of the 2D array s by 2, for every one time that it is called.

Below is the full code in case it is helpful to see how I'm using these things.

backend.ml

type sim = int array array

let create m n = Array.make m (Array.make n 0)

let update s = 
  for i=0 to ((Array.length s) - 2) do 
    for j=0 to (Array.length (s.(i))) - 2 do 
      (s.(i)).(j) <- (s.(i).(j)) + 2;
    done;
  done

let toString s =
  let rows = Array.length s in 
  if rows = 0 then "--\n||\n--"
  else
    let cols = Array.length s.(0) in 
    let st = ref "--\n" in
    for i=0 to rows-1 do 
      for j=0 to cols-1 do 
        st := !st ^ (string_of_int s.(i).(j)) ^ ","
      done;
      st := !st ^ "\n"
    done;
    !st 

main.ml

open Test.Backend

let sim = create 
          (int_of_string Sys.argv.(1)) 
          (int_of_string Sys.argv.(2))

;;
while true do
  print_endline (toString sim);
  update sim;
  Unix.sleepf 1.;
done

The print-out from running this with command-line arguments 3 and 10 looks like the following.

--                                 
0,0,0,0,0,0,0,0,0,0,
0,0,0,0,0,0,0,0,0,0,
0,0,0,0,0,0,0,0,0,0,

--
2,2,2,2,2,2,2,2,2,0,
2,2,2,2,2,2,2,2,2,0,
2,2,2,2,2,2,2,2,2,0,

--
4,4,4,4,4,4,4,4,4,0,
4,4,4,4,4,4,4,4,4,0,
4,4,4,4,4,4,4,4,4,0,

I cannot even guess at why it's doing this. I threw around a bunch of parentheses, tweaked a few numbers, just to see if it would change anything. I also don't understand why this ever updates the final row since I tried subtracting "too much". I'm guessing this is somehow related to the problem of adding too many times. But as I try to logically parse it out, I just cannot see how this is happening.


After tinkering, I threw in a print line in the update function and found that the update function is getting called twice for every one time that the while-loop executes. So now I don't understand why that is happening, but it's progress!

Upvotes: 0

Views: 62

Answers (1)

Chris
Chris

Reputation: 36620

Your problem lies here:

let create m n = Array.make m (Array.make n 0)

You've made an array with m references to the same array with n zeroes, because Array.make n 0 is only called once.

Consider:

# let m = Array.make 2 (Array.make 2 2);;
val m : int array array = [|[|2; 2|]; [|2; 2|]|]
# m.(0).(0) <- 8;;
- : unit = ()
# m;;
- : int array array = [|[|8; 2|]; [|8; 2|]|]

You likely want to use Array.make_matrix.

# let m = Array.make_matrix 2 2 2;;
val m : int array array = [|[|2; 2|]; [|2; 2|]|]
# m.(0).(0) <- 8;;
- : unit = ()
# m;;
- : int array array = [|[|8; 2|]; [|2; 2|]|]

You could also use Array.init to ensure you have an array of unique arrays.

# let m = Array.init 2 (fun _ -> Array.make 2 2);;
val m : int array array = [|[|2; 2|]; [|2; 2|]|]
# m.(0).(0) <- 8;;
- : unit = ()
# m;;
- : int array array = [|[|8; 2|]; [|2; 2|]|]

Building your string

As an added suggestion, you may wish to use the Buffer module when creating your string to output.

let toString s =
  let rows = Array.length s in 
  if rows = 0 then "--\n||\n--"
  else
    let buf = Buffer.create 16 in
    let cols = Array.length s.(0) in 
    Buffer.add_string buf "--\n";
    for i=0 to rows-1 do 
      for j=0 to cols-1 do 
        Buffer.add_string buf @@ string_of_int s.(i).(j);
        Buffer.add_string buf ","
      done;
      Buffer.add_string buf "\n"
    done;
    Buffer.contents buf

It wouldn't be a bad idea either to build a custom formatting function as shown below. This is much more flexible, but the logic follows what you've already created.

# let sim_pp fmt s =
    let rows = Array.length s in
    if rows = 0 then
      Format.fprintf fmt "--\n||\n--"
    else (
      let cols = Array.length s.(0) in
      Format.fprintf fmt "--\n";
      for i = 0 to rows - 1 do
        for j = 0 to cols - 1 do
          Format.fprintf fmt "%d," s.(i).(j)
        done;
        Format.fprintf fmt "\n"
      done
    );;
val sim_pp : Format.formatter -> int array array -> unit = <fun>
# Format.printf "%a\n" sim_pp [|[|1; 2|]; [|3; 4|]|];;
--
1,2,
3,4,

- : unit = ()
# Format.asprintf "%a\n" sim_pp [|[|1; 2|]; [|3; 4|]|];;
- : string = "--\n1,2,\n3,4,\n\n"
# [|[|1; 2|]; [|3; 4|]|]
  |> Format.asprintf "%a\n" sim_pp
  |> print_string;;
--
1,2,
3,4,

- : unit = ()

Parentheses

A good review of the operator precedence rules will show that the following has far more parentheses than are necessary.

type sim = int array array

let create m n =
  Array.make m (Array.make n 0)

let update s =    
  for i=0 to ((Array.length s) - 2) do 
    for j=0 to (Array.length (s.(i))) - 2 do 
      (s.(i)).(j) <- (s.(i).(j)) + 2;
    done;   done

let toString s =
  let rows = Array.length s in    if rows = 0 then
"--\n||\n--"   else
    let cols = Array.length s.(0) in 
    let st = ref "--\n" in
    for i=0 to rows-1 do 
      for j=0 to cols-1 do 
        st := !st ^ (string_of_int s.(i).(j)) ^ ","
      done;
      st := !st ^ "\n"
    done;
    !st

It can be reduced to:

type sim = int array array

let create m n = 
  Array.make m (Array.make n 0)

let update s = 
  for i=0 to Array.length s - 2 do 
    for j=0 to Array.length s.(i) - 2 do 
      s.(i).(j) <- s.(i).(j) + 2;
    done;
  done

let toString s =
  let rows = Array.length s in 
  if rows = 0 then "--\n||\n--"
  else
    let cols = Array.length s.(0) in 
    let st = ref "--\n" in
    for i=0 to rows-1 do 
      for j=0 to cols-1 do 
        st := !st ^ string_of_int s.(i).(j) ^ ","
      done;
      st := !st ^ "\n"
    done;
    !st

Format.pp_print_array

The above printing for an array of arrays might be written using the existing Format.pp_print_array function, rather than reinventing the wheel.

# Format.(
    let pp_print_comma fmt () = fprintf fmt "," in
    printf "%a\n"
      (pp_print_array
         ~pp_sep: pp_print_newline
         (pp_print_array ~pp_sep: pp_print_comma pp_print_int))
       [|[|2; 3|]; [|1; 4|]|]
  );;
2,3
1,4
- : unit = ()

Upvotes: 1

Related Questions