From 7fffa6886a4c3a8d4ca5681e3162870a1bbac68f Mon Sep 17 00:00:00 2001 From: Stuart Daines Date: Mon, 16 Dec 2024 16:21:44 +0000 Subject: [PATCH 1/2] Simplify thread-safe configuration Requires https://github.com/PALEOtoolkit/PALEOboxes.jl/pull/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 --- src/Initialize.jl | 23 +++++++++++------------ src/ODEfixed.jl | 11 ++++++----- test/runfieldtests.jl | 12 ++++++------ 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/Initialize.jl b/src/Initialize.jl index 6c6d074..9398c33 100644 --- a/src/Initialize.jl +++ b/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) diff --git a/src/ODEfixed.jl b/src/ODEfixed.jl index 9a248f2..2fb823e 100644 --- a/src/ODEfixed.jl +++ b/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 || diff --git a/test/runfieldtests.jl b/test/runfieldtests.jl index 6ffa644..dc37ded 100644 --- a/test/runfieldtests.jl +++ b/test/runfieldtests.jl @@ -48,7 +48,7 @@ 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 @@ -56,7 +56,7 @@ 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 @@ -64,7 +64,7 @@ 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) From 254e7301b1429f14ef2ff0c7a4ac7e309a454969 Mon Sep 17 00:00:00 2001 From: sjdaines Date: Mon, 16 Dec 2024 19:10:45 +0000 Subject: [PATCH 2/2] Update Project.toml [compat] for PALEOboxes --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index 3a8cf94..88b35cd 100644 --- a/Project.toml +++ b/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"