Return nothing from mutating BMI functions

docs/src/changelog.md

@@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Initialization of `LateralSSF` variables `ssf` and `ssfmax` with vertical hydraulic
   conductivity profile `exponential_constant`. Removed parameter `khfrac` from the
   computation, as it is already part of parameter `kh_0`.
+- Mutating BMI model control functions (`update`, `update_until` and `finalize`) and
+  extended mutating BMI functions (`load_state` and `save_state`) should return `nothing`.
 
 ### Changed
 - Removed vertical concepts `HBV` and `FLEXTopo`.

server/src/bmi_service.jl

@@ -227,13 +227,13 @@ function wflow_bmi(m::GetCurrentTime, model::Wflow.Model)
 end
 
 function wflow_bmi(m::UpdateUntil, model::Wflow.Model)
-    model = getfield(Wflow.BMI, Symbol(m.fn))(model, m.time)
-    return model
+    getfield(Wflow.BMI, Symbol(m.fn))(model, m.time)
+    return nothing
 end
 
 function wflow_bmi(m::Update, model::Wflow.Model)
-    model = getfield(Wflow.BMI, Symbol(m.fn))(model)
-    return model
+    getfield(Wflow.BMI, Symbol(m.fn))(model)
+    return nothing
 end
 
 function wflow_bmi(m::GetInputVarNames, model::Wflow.Model)
@@ -347,8 +347,8 @@ function wflow_bmi(m::Finalize, model::Wflow.Model)
 end
 
 function wflow_bmi(m::LoadState, model::Wflow.Model)
-    model = getfield(Wflow, Symbol(m.fn))(model)
-    return model
+    getfield(Wflow, Symbol(m.fn))(model)
+    return nothing
 end
 
 function wflow_bmi(m::SaveState, model::Wflow.Model)

server/src/server.jl

@@ -82,10 +82,10 @@ if required, depending on return type of `wflow.bmi(f, handler.model)`.
 function wflow_bmi(s::ZMQ.Socket, handler::ModelHandler, f)
     try
         ret = wflow_bmi(f, handler.model)
-        if typeof(ret) <: Wflow.Model # update of Wflow model
+        if typeof(ret) <: Wflow.Model # initialize Wflow model
             handler.model = ret
             response(s)
-        elseif isnothing(ret) # for SetValue and SetValueAtIndices
+        elseif isnothing(ret) # for mutating BMI functions (e.g. update, update_until and set_value)
             response(s)
         else
             @info "Send response including output from Wflow function `$(f.fn)`"

src/bmi.jl

@@ -55,7 +55,7 @@ function BMI.update(model::Model; run = nothing)
     elseif run == "sbm_after_subsurfaceflow"
         run_timestep!(model; update_func = update_after_subsurfaceflow!)
     end
-    return model
+    return nothing
 end
 
 function BMI.update_until(model::Model, time::Float64)
@@ -74,7 +74,7 @@ function BMI.update_until(model::Model, time::Float64)
     for _ in 1:steps
         run_timestep!(model)
     end
-    return model
+    return nothing
 end
 
 "Write state output to netCDF and close files."
@@ -85,7 +85,8 @@ function BMI.finalize(model::Model)
         write_netcdf_timestep(model, writer.state_dataset, writer.state_parameters)
     end
     reset_clock!(model.clock, config)
-    return close_files(model; delete_output = false)
+    close_files(model; delete_output = false)
+    return nothing
 end
 
 function BMI.get_component_name(model::Model)
@@ -401,7 +402,7 @@ end
 # May also be useful for other external software packages.
 function load_state(model::Model)
     set_states!(model)
-    return model
+    return nothing
 end
 
 function save_state(model::Model)
@@ -410,7 +411,8 @@ function save_state(model::Model)
         @info "Write output states to netCDF file `$(model.writer.state_nc_path)`."
     end
     write_netcdf_timestep(model, writer.state_dataset, writer.state_parameters)
-    return close(writer.state_dataset)
+    close(writer.state_dataset)
+    return nothing
 end
 
 function get_start_unix_time(model::Model)

test/bmi.jl

@@ -47,7 +47,7 @@ tomlpath = joinpath(@__DIR__, "sbm_config.toml")
             ) BMI.get_var_itemsize(model, "lateral.land.alpha_pow")
         end
 
-        model = BMI.update(model)
+        BMI.update(model)
 
         @testset "update and get and set functions" begin
             @test BMI.get_current_time(model) == 86400.0
@@ -126,12 +126,14 @@ tomlpath = joinpath(@__DIR__, "sbm_config.toml")
 
         @testset "update until and finalize" begin
             time = BMI.get_current_time(model) + 2 * BMI.get_time_step(model)
-            model = BMI.update_until(model, time)
+            BMI.update_until(model, time)
             @test model.clock.iteration == 3
             time_off = BMI.get_current_time(model) + 1 * BMI.get_time_step(model) + 1e-06
-            @test_throws ErrorException model = BMI.update_until(model, time_off)
-            @test_throws ErrorException model =
-                BMI.update_until(model, time - BMI.get_time_step(model))
+            @test_throws ErrorException BMI.update_until(model, time_off)
+            @test_throws ErrorException BMI.update_until(
+                model,
+                time - BMI.get_time_step(model),
+            )
             BMI.finalize(model)
         end
     end
@@ -158,7 +160,7 @@ tomlpath = joinpath(@__DIR__, "sbm_config.toml")
         model = BMI.initialize(Wflow.Model, tomlpath)
 
         # update the recharge part of the SBM model
-        model = BMI.update(model; run = "sbm_until_recharge")
+        BMI.update(model; run = "sbm_until_recharge")
 
         @testset "recharge part of SBM" begin
             sbm = model.vertical
@@ -181,7 +183,7 @@ tomlpath = joinpath(@__DIR__, "sbm_config.toml")
             fill(1.0e-5, BMI.get_grid_node_count(model, 6)),
         )
         # update SBM after subsurface flow
-        model = BMI.update(model; run = "sbm_after_subsurfaceflow")
+        BMI.update(model; run = "sbm_after_subsurfaceflow")
 
         @testset "SBM after subsurface flow" begin
             sbm = model.vertical
@@ -205,7 +207,7 @@ end
     @test Wflow.get_start_unix_time(model) == 9.466848e8
     satwaterdepth = mean(model.vertical.soil.variables.satwaterdepth)
     model.config.model.reinit = false
-    model = Wflow.load_state(model)
+    Wflow.load_state(model)
     @test satwaterdepth ≠ mean(model.vertical.soil.variables.satwaterdepth)
     @test_logs (
         :info,