Made sure that the model RNG is used throughout

Using the GLOBAL_RNG introduces global state, which must be avoided
to preserve reproducibility. Therefore, all Persephone code must use
`model.rng` whenever calling `rand()`/`shuffle!()`/etc.

Project.toml

@@ -14,3 +14,7 @@ Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
 StableRNGs = "860ef19b-820b-49d6-a774-d7a799459cd3"
 StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"
 TOML = "fa267f1f-6049-4f14-aa54-33bafae1ed76"
+
+[compat]
+julia = ">= 1.9"
+Agents = ">= 5.6"
\ No newline at end of file

src/Persephone.jl

@@ -52,8 +52,10 @@ export
     @distanceto,
     @distancetoedge,
     @countanimals,
+    @rand,
     #functions
     simulate,
+    simulate!,
     initialise,
     stepsimulation!,
     createevent!,

src/core/input.jl

@@ -53,7 +53,8 @@ function getsettings(configfile::String, seed::Union{Int64,Nothing}=nothing)
     # pre-process certain parameters
     if !isnothing(seed)
         settings["core"]["seed"] = seed
-    elseif settings["core"]["seed"] == 0
+    end
+    if settings["core"]["seed"] == 0
         settings["core"]["seed"] = abs(rand(RandomDevice(), Int32))
     end
     defaultoutdir = defaults["core"]["outdir"]
@@ -74,7 +75,7 @@ Certain software parameters can be set via the commandline.
 """
 function parsecommandline()
     versionstring = """
-            Persephone $(@project_version), commit $(read(`git rev-parse HEAD`, String)[1:8])
+            Persephone $(pkgversion(Persephone))
             © 2022-2023 Daniel Vedder, Lea Kolb (MIT license)
             https://git.idiv.de/xo30xoqa/persephone
             """
@@ -96,6 +97,7 @@ function parsecommandline()
             arg_type = String
             required = false
     end
+    #XXX this changes the global RNG?! (https://github.com/carlobaldassi/ArgParse.jl/issues/121)
     args = parse_args(s)
     for a in keys(args)
         (args[a] == nothing) && delete!(args, a)

src/core/simulation.jl

@@ -72,14 +72,23 @@ function finalise!(model::AgentBasedModel)
 end
 
 """
-    simulate(config=PARAMFILE, seed=nothing)
+    simulate!(model)
 
-Carry out a complete simulation run, optionally specifying a configuration file
-and a seed for the RNG.
+Carry out a complete simulation run using a pre-initialised model object.
 """
-function simulate(config::String=PARAMFILE, seed::Union{Int64,Nothing}=nothing)
-    model = initialise(config, seed)
+function simulate!(model::AgentBasedModel)
     runtime = Dates.value(@param(core.enddate)-@param(core.startdate))+1
     step!(model, dummystep, stepsimulation!, runtime)
     finalise!(model)
 end
+
+"""
+    simulate(config=PARAMFILE, seed=nothing)
+
+Initialise a model object and carry out a complete simulation run, optionally
+specifying a configuration file and a seed for the RNG.
+"""
+function simulate(config::String=PARAMFILE, seed::Union{Int64,Nothing}=nothing)
+    model = initialise(config, seed)
+    simulate!(model)
+end

src/crop/crops.jl

@@ -52,7 +52,7 @@ function initfields!(model::AgentBasedModel)
                 model.landscape[x,y].fieldid = objectid
                 push!(model[objectid].pixels, (x,y))
             else
-                fp = add_agent!(FarmPlot, model, [(x,y)], fallow, 0.0, 0.0)
+                fp = add_agent!((x,y), FarmPlot, model, [(x,y)], fallow, 0.0, 0.0)
                 model.landscape[x,y].fieldid = fp.id
                 convertid[rawid] = fp.id
                 n += 1

src/nature/nature.jl

@@ -372,7 +372,9 @@ Return a random number or element from the sample, using the model RNG.
 This is a utility wrapper that can only be used nested within `@phase` or `@habitat`.
 """
 macro rand(args...)
-    :(rand($(esc(:model)).rng, $(map(esc, args)...)))
+    :($(esc(:rand))($(esc(:model)).rng, $(map(esc, args)...)))
 end
 
+##TODO @chance macro: @chance(0.5) => rand(model.rng) < 0.5
+
 ##TODO add movement macros

src/nature/species/wolpertinger.jl

@@ -24,7 +24,7 @@ of a deer.
     and occasionally reproduce by spontaneous parthogenesis...
     """
     @phase lifephase begin
-        direction = Tuple(rand([-1,1], 2))
+        direction = Tuple(@rand([-1,1], 2))
         for i in 1:@rand(1:@trait(maxspeed))
             walk!(animal, direction, model; ifempty=false)
         end

test/io_tests.jl

@@ -4,9 +4,11 @@
 ###
 
 @testset "Model configuration" begin
-    properties = Dict{Symbol,Any}(:settings=>TESTSETTINGS)
+    settings = Ps.getsettings(TESTPARAMETERS)
+    properties = Dict{Symbol,Any}(:settings=>settings)
     space = GridSpace((10,10), periodic=false)
     model = AgentBasedModel(Animal, space, properties=properties, warn=false)
+
     @test @param(core.configfile) == TESTPARAMETERS
     @test @param(core.startdate) == Date(2022, 2, 1)
     @test @param(nature.targetspecies) == ["Wolpertinger", "Wyvern"]

test/landscape_tests.jl

@@ -30,7 +30,8 @@ function smalltestlandscape(agenttype::Type=Animal)
                                   :events=>Vector{FarmEvent}(),
                                   :dataoutputs=>Vector{DataOutput}(),
                                   :settings=>TESTSETTINGS)
-    return AgentBasedModel(agenttype, space, properties=properties, warn=false)
+    return AgentBasedModel(agenttype, space, properties=properties,
+                           rng=StableRNG(TESTSETTINGS["core"]["seed"]), warn=false)
 end
 
 @testset "Landscape initialisation" begin

test/nature_tests.jl

@@ -68,15 +68,15 @@ end
 
 @testset "Species macros" begin
     # create a model landscape and a test species
-    #TODO test @rand
     model = smalltestlandscape(Union{Animal,Farmer,FarmPlot})
+    
     @species Mermaid begin
         ageofmaturity = 2
         pesticidemortality = 1.0
         @initialise(@habitat(@landcover() == Persephone.water), pairs=true)
         @phase life begin
             @debug "$(Persephone.animalid(animal)) is swimming happily in its pond."
-            @respond Persephone.pesticide @kill(@trait(pesticidemortality))
+            @respond Persephone.pesticide @kill(@trait(pesticidemortality), "poisoning")
             @respond Persephone.harvest @setphase(drought)
             @debug "Animal: $animal"
             if @trait(sex) == Persephone.female && @countanimals() < 3 &&
@@ -123,12 +123,16 @@ end
     @test Ps.countanimals(pond, model) == 3
     createevent!(model, [pond], Ps.pesticide)
     @test_logs((:debug, "Mermaid 1 is swimming happily in its pond."),
-               (:debug, "Mermaid 1 has died."),
+               (:debug, "Mermaid 1 has died from poisoning."),
                (:debug, "Mermaid 2 is swimming happily in its pond."),
-               (:debug, "Mermaid 2 has died."),
+               (:debug, "Mermaid 2 has died from poisoning."),
                (:debug, "Mermaid 3 is swimming happily in its pond."),
-               (:debug, "Mermaid 3 has died."),
+               (:debug, "Mermaid 3 has died from poisoning."),
                min_level=Logging.Debug, match_mode=:any,
                stepsimulation!(model))
     @test Ps.countanimals(pond, model) == 0
+
+    # test @rand (this is done more easily outside of @species)
+    @test typeof(@rand()) == Float64
+    @test @rand([true, true])
 end

test/runtests.jl

@@ -6,12 +6,13 @@
 using Pkg
 Pkg.activate("..")
 
+using Agents
+using Dates
+using Logging
 using Persephone
-using Test
 using Random
-using Logging
-using Dates
-using Agents
+using StableRNGs
+using Test
 
 const Ps = Persephone
 

test/simulation_tests.jl

@@ -15,19 +15,24 @@
 end
 
 @testset "Model simulation" begin
+    # The primary reason for this testset is to make sure that a complete
+    # simulation will run through without errors. Thus, there are few tests.
+    # Additionally, it makes sure that no part of the code uses the global
+    # RNG, as this would compromise reproducibility. If one of the `rand()`
+    # tests fail, that requirement has been violated somewhere.
+    Random.seed!(1)
+    rand1 = rand()
+    Random.seed!(1)
+    model = initialise(TESTPARAMETERS, 218)
+    #XXX upstream problem with ArgParse (https://github.com/carlobaldassi/ArgParse.jl/issues/121)
+    @test_broken rand() == rand1 
     Random.seed!(1)
-    @test isapprox(rand(), 0.07337, atol=0.0001)
-    model = simulate(TESTPARAMETERS, 218)
     @test @param(core.seed) == 218
+    @test_logs((:info, "Simulating day 2022-02-01."),
+               (:info, "Simulated 59 days."),
+               min_level=Logging.Debug, match_mode=:any,
+               simulate!(model))
     @test model.date == Date(2022,4,1)
-    nwol = sum((typeof(a)==Animal && a.traits["name"]=="Wolpertinger") for a in allagents(model))
-    nwyv = sum((typeof(a)==Animal && a.traits["name"]=="Wyvern") for a in allagents(model))
-    #FIXME these still fail - although it might not be that clever to rely on random model outcomes
-    @test nwol == 32
-    @test nwyv == 9
-    # To retain reproducibility, the model code must never use the global RNG.
-    # If this next test fails, that requirement has probably been violated somewhere.
-    #FIXME it does fail... Might it be called by some dependency?
-    @test isapprox(rand(), 0.34924, atol=0.0001)
+    @test rand() == rand1
 end