Simplify thread-safe configuration (#105)

* Simplify thread-safe configuration

Requires PALEOtoolkit/PALEOboxes.jl#147

Remove 'initialize!' 'threadsafe' parameter.

Thread-safe configuration is now specified by a combination of:
- 'thread_safe: true' top-level parameter in YAML file
- setting 'method_barrier' keyword argument

* Update Project.toml [compat] for PALEOboxes

Project.toml

@@ -41,7 +41,7 @@ JLD2 = "0.4, 0.5"
 MultiFloats = "1.0, 2.0"
 NCDatasets = "0.12, 0.14.2"
 NLsolve = "4.5"
-PALEOboxes = "0.21.31"
+PALEOboxes = "0.21.37"
 RecipesBase = "1.2"
 Requires = "1.0"
 Revise = "3.1"

src/Initialize.jl

@@ -21,16 +21,21 @@ the `:datatype` Variable attribute to specify `String`-valued tags, in combinati
 provide a `Dict` of tag names => `DataType`s. 
 
 # Thread safety
-A thread-safe model can be created with `threadsafe=true` (to create Atomic Variables for those Variables with attribute `:atomic==true`),
-and supplying `method_barrier` (a thread barrier to add to `ReactionMethod` dispatch lists between dependency-free groups)
+A thread-safe model can be created by setting the `threadsafe=true` global parameter in the YAML config file before calling `PB.create_model_from_config` 
+(used by Reactions that require eg special handling of global accumulator variables to maintain thread safety),
+and supplying `method_barrier` (a thread barrier to add to `ReactionMethod` dispatch lists between dependency-free groups):
+
+    method_barrier = PB.reaction_method_thread_barrier(
+        PALEOmodel.ThreadBarriers.ThreadBarrierAtomic("the barrier"),
+        PALEOmodel.ThreadBarriers.wait_barrier
+    )
 
 # Keyword summary
 - `pickup_output=nothing`: OutputWriter with pickup data to initialise from
 - `check_units_opt=:no`: check linked Variable units are consistent (:no to disable check, :warn to warn and continue, :error to error and stop)
 - `eltype::Type=Float64`: default data type to use for model arrays
 - `eltypemap=Dict{String, DataType}()`: Dict of data types to look up Variable :datatype attribute
-- `threadsafe=false`: true to create thread safe Atomic Variables where Variable attribute `:atomic==true`
-- `method_barrier=nothing`: thread barrier to add to dispatch lists if `threadsafe==true`
+- `method_barrier=nothing`: thread barrier to add to dispatch lists to create a thread-safe model
 - `expect_hostdep_varnames=["global.tforce"]`: non-state-Variable host-dependent Variable names expected
 - `SolverView_all=true`: `true` to create `modeldata.solver_view_all`
 - `create_dispatchlists_all=true`: `true` to create `modeldata.dispatchlists_all`
@@ -42,13 +47,7 @@ function initialize!(
     check_units_opt=:no,
     eltype=Float64,
     eltypemap=Dict{String, DataType}(),     
-    threadsafe=false,
-    method_barrier=threadsafe ? 
-        PB.reaction_method_thread_barrier(
-            PALEOmodel.ThreadBarriers.ThreadBarrierAtomic("the barrier"),
-            PALEOmodel.ThreadBarriers.wait_barrier
-        ) : 
-        nothing,
+    method_barrier=nothing,
     expect_hostdep_varnames=["global.tforce"],
     SolverView_all=true,
     create_dispatchlists_all=true,
@@ -64,7 +63,7 @@ function initialize!(
     # check Variable linking early and exit if problems, so user sees appropriate error not a downstream consequence when allocating variable etc
     PB.check_variable_links(model; throw_on_error=true, expect_hostdep_varnames)
 
-    modeldata = PB.create_modeldata(model, eltype; threadsafe)
+    modeldata = PB.create_modeldata(model, eltype)
 
     # Allocate variables
     @timeit "allocate_variables" PB.allocate_variables!(model, modeldata, 1; eltypemap, check_units_opt)

src/ODEfixed.jl

@@ -165,9 +165,9 @@ function integrateEulerthreads(
     PB.check_modeldata(run.model, modeldata)
 
     nt = Threads.nthreads()
-    nt == 1 || modeldata.threadsafe || 
-        error("integrateEulerthreads: Threads.nthreads() = $nt but modeldata is not thread safe "*
-            "(check initialize!(run.model, ...))")
+    nt == 1 || get(modeldata.model.parameters, "threadsafe", false)  || 
+        error("integrateEulerthreads: Threads.nthreads() = $nt but model is not thread safe "*
+            "(set 'threadsafe=true' in YAML config top-level 'parameters:')")
 
     lc = length(cellranges)
     lc == nt ||
@@ -242,8 +242,9 @@ function integrateSplitEulerthreads(
     PB.check_modeldata(run.model, modeldata)
 
     nt = Threads.nthreads()
-    nt == 1 || modeldata.threadsafe || 
-        error("integrateEulerthreads: Threads.nthreads() = $nt but modeldata is not thread safe (check initialize!(run::Run, ...))")
+    nt == 1 || get(modeldata.model.parameters, "threadsafe", false)  || 
+        error("integrateSplitEulerthreads: Threads.nthreads() = $nt but model is not thread safe "*
+            "(set 'threadsafe=true' in YAML config top-level 'parameters:')")
 
     lc_outer = length(cellranges_outer)
     lc_outer == nt || 

test/runfieldtests.jl

@@ -48,23 +48,23 @@ end
 
     @testset "ScalarData, ScalarSpace" begin
 
-        f = PB.allocate_field(PB.ScalarData, (), Float64, PB.ScalarSpace, nothing; thread_safe=false, allocatenans=true)
+        f = PB.allocate_field(PB.ScalarData, (), Float64, PB.ScalarSpace, nothing; allocatenans=true)
 
         test_scalar_field(f)
     end
 
     @testset "ScalarData, CellSpace, no grid" begin
         # check that a CellSpace Field with no grid behaves as a ScalarSpace Field
 
-        f = PB.allocate_field(PB.ScalarData, (), Float64, PB.CellSpace, nothing; thread_safe=false, allocatenans=true)
+        f = PB.allocate_field(PB.ScalarData, (), Float64, PB.CellSpace, nothing; allocatenans=true)
 
         test_scalar_field(f)
     end
 
     @testset "ScalarData, CellSpace, UnstructuredColumnGrid" begin
         g = PB.Grids.UnstructuredColumnGrid(ncells=5, Icolumns=[collect(1:5)])
 
-        f = PB.allocate_field(PB.ScalarData, (), Float64, PB.CellSpace, g; thread_safe=false, allocatenans=true)
+        f = PB.allocate_field(PB.ScalarData, (), Float64, PB.CellSpace, g; allocatenans=true)
 
         f.values .= 42.0
         fr = PALEOmodel.FieldRecord(f, Dict{Symbol, Any}(), coords_record=[])
@@ -86,7 +86,7 @@ end
     end
 
     @testset "ArrayScalarData, ScalarSpace" begin
-        f = PB.allocate_field(PB.ArrayScalarData, (PB.NamedDimension("test", 2, []), ), Float64, PB.ScalarSpace, nothing; thread_safe=false, allocatenans=true)
+        f = PB.allocate_field(PB.ArrayScalarData, (PB.NamedDimension("test", 2, []), ), Float64, PB.ScalarSpace, nothing; allocatenans=true)
 
         @test isnan.(f.values) == [true, true]
         f.values .= [42.0, 43.0]
@@ -125,7 +125,7 @@ end
     @testset "ArrayScalarData, CellSpace, no grid" begin
         # TODO this is possibly inconsistent with (ScalarData, CellSpace, no grid),
         # as Field.values here is a (1, 2) Array, not a (2,) Vector
-        f = PB.allocate_field(PB.ArrayScalarData, (PB.NamedDimension("test", 2, []), ), Float64, PB.CellSpace, nothing; thread_safe=false, allocatenans=true)
+        f = PB.allocate_field(PB.ArrayScalarData, (PB.NamedDimension("test", 2, []), ), Float64, PB.CellSpace, nothing; allocatenans=true)
 
         @test_broken size(f.values) == (2, ) # TODO should be a Vector ?
         @test size(f.values) == (1, 2)
@@ -159,7 +159,7 @@ end
     @testset "ArrayScalarData, CellSpace, UnstructuredColumnGrid" begin
         g = PB.Grids.UnstructuredColumnGrid(ncells=5, Icolumns=[collect(1:5)])
 
-        f = PB.allocate_field(PB.ArrayScalarData, (PB.NamedDimension("test", 2, []), ), Float64, PB.CellSpace, g; thread_safe=false, allocatenans=true)
+        f = PB.allocate_field(PB.ArrayScalarData, (PB.NamedDimension("test", 2, []), ), Float64, PB.CellSpace, g; allocatenans=true)
 
         @test size(f.values) == (5, 2)