Request for code critique/reivew for first time learner coming from Rust

I decided that I’d learn OCaml by doing something basic I’m familiar with, so I wrote this file for solving a mass on a spring, and I wanted to ask if people could provide any critique/suggestions for improvement/style changes as this is literally the first program I’ve written.

I’m using a module because I want to get familiar with them, so everything related to a simulation is in this module. I also defined a signature for a simulation type but I’m not sure how to use it because I’d need like 5 M:S with type kind of statements.

module Simulation = struct
  (* Simulation interface. idk how to use it tho. ideas?*)
  module type S = sig
    type err
    type state
    type params
    type history
    type algorithms

    val step : state -> params -> algorithms -> ( state, err ) result
    val step_append : state -> params -> algorithms -> history -> ( state, err ) result
    val init_history : int -> history
  end

This submodule implements Simulation.S (though if I do that the associated types become, idk is opaque the right word? so I can’t use it from outside the module). The state of the simulation at any point is a position and velocity, I have some parameters for the simulation, a array storing the past states, and a list of algorithms that can be used to step.

  (* SHO simulation impl *)
  module SHO = struct
    type err = string
    type state = {
      x : float;
      v : float;
    }
    type params = {
      k: float;
      dt: float;
    }
    type history = {
      ptr: int ref;
      xs: float array;
      vs: float array;
      ts: float array;
    }

    type algorithms =
      | Euler
      | Leapfrog
      | RK2
      | RK4

I’m not sure if these functions are idiomatic ocaml. So I’d appreciate any feedback.

    let step ( s:state ) ( p:params ) ( m:algorithms ) = 
      match m with
        | Euler -> 
            Ok {
              x = s.x +. s.v *. p.dt;
              v = s.v +. (-. p.k *. s.x) *. p.dt;
            }
        | _ -> Error "unimplemented!" 

    let init_history n = {
      ptr = ref 0;
      xs = Array.create_float n;
      vs = Array.create_float n;
      ts = Array.create_float n;
    }

    let push_state (p:params) (h:history) (s:state) =
      let n = h.ptr.contents + 1 in
      if n >= Array.length h.ts then Error "no space left"
      else Ok (  
        h.ts.(n) <- (h.ts.(n-1) +. p.dt);
        h.xs.(n) <- s.x;
        h.vs.(n) <- s.v;
      )

    let step_append (s:state) (p:params) (m:algorithms) (h:history) =
      let s' = step s p m in
      if Result.is_error s' then s'
      else let s'' = Result.get_ok s' in
      match push_state p h s'' with
      | Ok () -> Ok s''
      | Error msg -> Error msg
  end
end

I suppose this block just depends on how the above code was written, but once again, any suggestions are welcome. Maybe better ways to do string formatting or printing of records?

let () =
  Simulation.SHO.(
    let h = init_history 10 in
    let rec loop s p m n =
      if n <= 0 then () else
      match ( step_append s p m h ) with
      | Ok s' -> (
          print_string "v = "; print_float s'.v; print_endline "";
          print_string "x = "; print_float s'.x; print_endline "";
          loop s' p m (n-1)
        )
      | Error msg -> print_string "error: "; print_endline msg
    in
    loop {x = 1.0; v = 0.0} {k = 1.0; dt = 0.001} Euler 15
  )

tysm for taking the time to read/comment <3

Copy-pastable code in one chunk
module Simulation = struct
  (* Simulation interface. idk how to use it tho. ideas?*)
  module type S = sig
    type err
    type state
    type params
    type history
    type algorithms

    val step : state -> params -> algorithms -> ( state, err ) result
    val step_append : state -> params -> algorithms -> history -> ( state, err ) result
    val init_history : int -> history
  end

  (* SHO simulation impl *)
  module SHO = struct
    type err = string
    type state = {
      x : float;
      v : float;
    }
    type params = {
      k: float;
      dt: float;
    }
    type history = {
      ptr: int ref;
        xs: float array;
        vs: float array;
        ts: float array;
    }

    type algorithms =
      | Euler
      | Leapfrog
      | RK2
      | RK4

    let step ( s:state ) ( p:params ) ( m:algorithms ) = 
      match m with
        | Euler -> 
            Ok {
              x = s.x +. s.v *. p.dt;
              v = s.v +. (-. p.k *. s.x) *. p.dt;
            }
        | _ -> Error "unimplemented!" 

    let init_history n = {
      ptr = ref 0;
      xs = Array.create_float n;
      vs = Array.create_float n;
      ts = Array.create_float n;
    }

    let push_state (p:params) (h:history) (s:state) =
      let n = h.ptr.contents + 1 in
      if n >= Array.length h.ts then Error "no space left"
      else Ok (  
        h.ts.(n) <- (h.ts.(n-1) +. p.dt);
        h.xs.(n) <- s.x;
        h.vs.(n) <- s.v;
      )

    let step_append (s:state) (p:params) (m:algorithms) (h:history) =
      let s' = step s p m in
      if Result.is_error s' then s'
      else let s'' = Result.get_ok s' in
      match push_state p h s'' with
      | Ok () -> Ok s''
      | Error msg -> Error msg
  end
end

let () =
  Simulation.SHO.(
    let h = init_history 10 in
    let rec loop s p m n =
      if n <= 0 then () else
      match ( step_append s p m h ) with
      | Ok s' -> (
          print_string "v = "; print_float s'.v; print_endline "";
          print_string "x = "; print_float s'.x; print_endline "";
          loop s' p m (n-1)
        )
      | Error msg -> print_string "error: "; print_endline msg
    in
    loop {x = 1.0; v = 0.0} {k = 1.0; dt = 0.001} Euler 15
  )

Hello @allonsyeet, welcome to the forum!

Your code looks quite good at a glance. A few suggestions/remarks (opinions below, caveat emptor).

One of the purposes of modules is to serve as a structuring device for large-scale programs, and they can be awkward to use in small programs. My suggestion to beginners it always to concentrate on the bread and butter of OCaml: functions, datatypes, pattern matching. That subset of OCaml is simple and powerful, and can get you surprisingly far. You can broach modules once you have a clear need for them :slight_smile:

A few comments:

  • The field ptr can be declared as mutable instead of using a int ref. The difference is subtle but important semantically: an int ref introduces an indirection; it is a value that exists on its own and can therefore be shared by different parts of the code, which you probably want to avoid as it means more chances of “action at a distance”. Using a mutable field instead allows mutability without introducing the extra indirection, which is both more efficient and reduces the possible set of side-effects in your code.

  • You annotate function parameters with their types. There is nothing wrong about this, but with experience OCaml programmers typically tend to elide the type annotations (unless they are needed for disambiguation purposes). The reason is that it makes it simpler to refactor the code: no need to update the annotations when the type of arguments changes.

  • In push_state you have Ok (e1; e2; e3). There is nothing wrong with this, but one would typically write begin e1; e2; e3; Ok () end which more clearly shows the structure of the code.

  • In step_append you switch cases using Result.is_error, Result.get_ok, etc. Instead you should use pattern matching

    match s' with
    | Error _ as err -> err
    | Ok s'' -> ...
    

    Using pattern matching provides the compiler with more information about the intent of your code, and the compiler can help you in return: warn you if you have not handled all possible cases, if you have useless overlapping cases, etc. In short: pattern matching should be the first tool you reach for in most circumstances.

Incidentally, it is refreshing to see people still try to properly learn a language instead of oursourcing understanding to an AI.

Cheers,
Nicolas

Hiya! tysm for the deailed response!

So I’ve updated the type history to this

type history = {
  mutable ptr: int;
  xs: float array;
  vs: float array;
  ts: float array;
}

So my reasoning for that is otherwise the function becomes harder to read without something like an LSP that can tell you the type of the function because the variable names are just single letters, though I suppose that can be fixed with better naming or a comment nearby.

That makes much more sense! I did not know about the begin ... end syntax.

I rewrote the function like so (haven’t removed the type annotations yet)

let step_append (s:state) (p:params) (m:algorithms) (h:history) =
  let s' = step s p m in
  match s' with
  | Ok s'' -> (
    match push_state p h s'' with
    | Ok () -> Ok s''
    | Error _ as err -> err
  )
  | Error _ as err -> err

Since the idiom of if Ok do X else return error keeps repeating, I was wondering does OCaml have syntax for early returning in case of an error variant like Zig’s try or Rust’s ? operator? I found out about binding operators and I know the corresponding operators are in Result.Syntax but I’m not completely sure how to use them yet. I don’t want to open the module globally and I don’t know how to do it in a function scope.

Once again, thank you for the detailed response, really appreciate it :slight_smile: .

Edit: Okay just learned about the let open ... in syntax so I came up with this.

let step_append (s:state) (p:params) (m:algorithms) (h:history) =
  let open Result.Syntax in
  let* s' = step s p m in
  match push_state p h s' with
  | Ok () -> Ok s'
  | Error _ as err -> err

Looked up the definition of let* operator and it’s the Result.bind operation which is:

let bind r f = match r with Ok v -> f v | Error _ as e -> e

Which is exactly the early return if I’m not wrong? The let* statement says take the result from step s p m, and if it’s Ok, apply the function fun s' -> match push_state p h s' with Ok () -> Ok s' | Error _ as err -> err to the inner state. As far as I understand.

Edit 2: I realized that step_append can be simplified even further since we have the binding operators

let step_append s p m h =
  let open Result.Syntax in
  let* s' = step s p m in
  let* _ = push_state p h s' in
  Ok s'

begin ... end is equivalent to parentheses, but is often used to delimit expression sequences (which are executed for side-effects).

Yep, that’s it!

Cheers,
Nicolas