From 6078618e7d19b2c26e2fe9e04e5aec2292d02541 Mon Sep 17 00:00:00 2001
From: Daniel Vedder <daniel.vedder@idiv.de>
Date: Tue, 10 Jan 2023 11:37:05 +0100
Subject: [PATCH] Finished rewriting parameter system.

This is a major change to the codebase. I'm not entirely convinced yet
that it actually makes sense, but overall I think the benefits
outweigh the problems. Closes issue #37.
---
 src/Persephone.jl            |  7 +-----
 src/core/input.jl            |  3 +--
 src/core/landscape.jl        |  8 +++----
 src/core/output.jl           | 46 ++++++++++++++++++------------------
 src/core/simulation.jl       | 42 +++++++++++++++++---------------
 src/nature/ecologicaldata.jl | 12 +++++-----
 src/nature/nature.jl         |  4 ++--
 test/io_tests.jl             | 31 ++++++++++++++----------
 test/landscape_tests.jl      | 22 +++++++++--------
 test/runtests.jl             |  4 ++--
 10 files changed, 92 insertions(+), 87 deletions(-)

diff --git a/src/Persephone.jl b/src/Persephone.jl
index eb7fd2b..43ef2f1 100644
--- a/src/Persephone.jl
+++ b/src/Persephone.jl
@@ -31,22 +31,17 @@ export
     Animal,
     Farmer,
     #macros
+    @param,
     @species,
     @phase,
     @habitat,
     #functions
-    param,
     simulate,
     initialise,
     stepsimulation!,
     createevent!,
     finalise
 
-## The file that stores all default parameters
-const PARAMFILE = "src/parameters.toml"
-## (DO NOT CHANGE THIS VALUE! Instead, specify simulation-specific configuration files
-## by using the "--configfile" commandline argument, or when invoking simulate().) 
-
 ## include all module files (note that the order matters - if file
 ## b references something from file a, it must be included later)
 include("core/input.jl")
diff --git a/src/core/input.jl b/src/core/input.jl
index ada3f58..7009844 100644
--- a/src/core/input.jl
+++ b/src/core/input.jl
@@ -21,7 +21,6 @@ object is available.
 macro param(domainparam)
     domain = String(domainparam.args[1])
     paramname = String(domainparam.args[2].value)
-    #domain, paramname = split(domainparam, ".")
     :($(esc(:model)).settings[$domain][$paramname])
 end
 
@@ -31,7 +30,7 @@ end
 Combines all configuration options to produce a single settings dict.
 Precedence: commandline parameters - user config file - default values
 """
-function getsettings(configfile::String, seed::Union{Int64,Nothing)=nothing)
+function getsettings(configfile::String, seed::Union{Int64,Nothing}=nothing)
     # read in and merge configurations from the commandline, the default config file
     # and a user-supplied config file
     defaults = TOML.parsefile(configfile)
diff --git a/src/core/landscape.jl b/src/core/landscape.jl
index 3ea00c3..a7f1085 100644
--- a/src/core/landscape.jl
+++ b/src/core/landscape.jl
@@ -36,15 +36,15 @@ mutable struct FarmEvent
 end
 
 """
-    initlandscape()
+    initlandscape(landcovermap, farmfieldsmap)
 
 Initialise the model landscape based on the map files specified in the
 configuration. Returns a matrix of pixels.
 """
-function initlandscape()
+function initlandscape(landcovermap::String, farmfieldsmap::String)
     @debug "Initialising landscape"
-    landcover = GeoArrays.read(param("core.landcovermap"))
-    farmfields = GeoArrays.read(param("core.farmfieldsmap"))
+    landcover = GeoArrays.read(landcovermap)
+    farmfields = GeoArrays.read(farmfieldsmap)
     (size(landcover) != size(farmfields)) && Base.error("Input map sizes don't match.")
     width, height = size(landcover)[1:2]
     landscape = Matrix{Pixel}(undef, width, height)
diff --git a/src/core/output.jl b/src/core/output.jl
index e34feb2..a75c347 100644
--- a/src/core/output.jl
+++ b/src/core/output.jl
@@ -13,12 +13,12 @@ const LOGFILE = "simulation.log"
 
 Creates the output directory and copies relevant files into it.
 """
-function setupdatadir()
+function setupdatadir(model::AgentBasedModel)
     # Check whether the output directory already exists and handle conflicts
-    if isdir(param("core.outdir"))
-        overwrite = param("core.overwrite")
-        if param("core.overwrite") == "ask"
-            println("The chosen output directory $(param("core.outdir")) already exists.")
+    if isdir(@param(core.outdir))
+        overwrite = @param(core.overwrite)
+        if @param(core.overwrite) == "ask"
+            println("The chosen output directory $(@param(core.outdir)) already exists.")
             println("Type 'yes' to overwrite this directory. Otherwise, the simulation will abort.")
             print("Overwrite? ")
             answer = readline()
@@ -27,39 +27,39 @@ function setupdatadir()
         if !overwrite
             Base.error("Output directory exists, will not overwrite. Aborting.")
         else
-            @warn "Overwriting existing output directory $(param("core.outdir"))."
+            @warn "Overwriting existing output directory $(@param(core.outdir))."
         end
     end
-    mkpath(param("core.outdir"))
+    mkpath(@param(core.outdir))
     # Setup the logging system and logfile
     loglevel = Logging.Info
-    if param("core.loglevel") == "debug"
+    if @param(core.loglevel) == "debug"
         loglevel = Logging.Debug
-    elseif param("core.loglevel") == "quiet"
+    elseif @param(core.loglevel) == "quiet"
         loglevel = Logging.Warn
     end
-    logfile = open(joinpath(param("core.outdir"), LOGFILE), "w+")
+    logfile = open(joinpath(@param(core.outdir), LOGFILE), "w+")
     simulationlogger = TeeLogger(ConsoleLogger(logfile, loglevel),
                                  ConsoleLogger(stdout, loglevel))
     global_logger(simulationlogger)
-    @debug "Setting up output directory $(param("core.outdir"))"
+    @debug "Setting up output directory $(@param(core.outdir))"
     # Export a copy of the current parameter settings to the output folder.
     # This can be used to replicate this exact run in future, and also
     # records the current time and git commit.
-    open(joinpath(param("core.outdir"), basename(param("core.configfile"))), "w") do f
+    open(joinpath(@param(core.outdir), basename(@param(core.configfile))), "w") do f
         println(f, "#\n# --- Persephone configuration parameters ---")
         println(f, "# This file was generated automatically.")
         println(f, "# Simulation run on $(string(Dates.format(Dates.now(), "d u Y HH:MM:SS"))),")
         println(f, "# with git commit $(read(`git rev-parse HEAD`, String))#\n")
-        TOML.print(f, settings)
+        TOML.print(f, model.settings)
     end
     # Copy the map files to the output folder
-    lcmap = param("core.landcovermap")
-    ffmap = param("core.farmfieldsmap")
+    lcmap = @param(core.landcovermap)
+    ffmap = @param(core.farmfieldsmap)
     !(isfile(lcmap)) && Base.error("The map file $(lcmap) doesn't exist.")
     !(isfile(ffmap)) && Base.error("The map file $(ffmap) doesn't exist.")
-    cp(lcmap, joinpath(param("core.outdir"), basename(lcmap)), force = true)
-    cp(ffmap, joinpath(param("core.outdir"), basename(ffmap)), force = true)
+    cp(lcmap, joinpath(@param(core.outdir), basename(lcmap)), force = true)
+    cp(ffmap, joinpath(@param(core.outdir), basename(ffmap)), force = true)
 end
 
 """
@@ -87,12 +87,12 @@ let outputregistry = Vector{DataOutput}(),
     nextyearlyoutput = today()
 
     """
-        newdataoutput(filename, header, outputfunction, frequency)
+        newdataoutput(model, filename, header, outputfunction, frequency)
 
     Create and register a new data output. This function must be called by all submodels
     that want to have their output functions called regularly.
     """
-    global function newdataoutput(filename::String, header::String,
+    global function newdataoutput(model::AgentBasedModel, filename::String, header::String,
                                   outputfunction::Function, frequency::String)
         if !(frequency in ("daily", "monthly", "yearly", "end", "never"))
             Base.error("Invalid frequency '$frequency' for $filename.")
@@ -100,7 +100,7 @@ let outputregistry = Vector{DataOutput}(),
         ndo = DataOutput(filename, header, outputfunction, frequency)
         append!(outputregistry, [ndo])
         if frequency != "never"
-            open(joinpath(param("core.outdir"), filename), "w") do f
+            open(joinpath(@param(core.outdir), filename), "w") do f
                 println(f, header)
             end
         end
@@ -115,7 +115,7 @@ let outputregistry = Vector{DataOutput}(),
     global function outputdata(model::AgentBasedModel)
         #XXX all output functions are run on the first update (regardless of frequency)
         # -> should they all be run on the last update, too?
-        if model.date == param("core.startdate")
+        if model.date == @param(core.startdate)
             nextmonthlyoutput = model.date
             nextyearlyoutput = model.date
         end
@@ -125,8 +125,8 @@ let outputregistry = Vector{DataOutput}(),
             if (output.frequency == "daily") ||
                 (output.frequency == "monthly" && model.date == nextmonthlyoutput) ||
                 (output.frequency == "yearly" && model.date == nextyearlyoutput) ||
-                (output.frequency == "end" && model.date == param("core.enddate"))
-                open(joinpath(param("core.outdir"), output.filename), "a") do f
+                (output.frequency == "end" && model.date == @param(core.enddate))
+                open(joinpath(@param(core.outdir), output.filename), "a") do f
                     outstring = output.outputfunction(model)
                     (outstring[end] != '\n') && (outstring *= '\n')
                     print(f, outstring)
diff --git a/src/core/simulation.jl b/src/core/simulation.jl
index d36aefa..11244c8 100644
--- a/src/core/simulation.jl
+++ b/src/core/simulation.jl
@@ -4,22 +4,25 @@
 ###
 
 """
-    initialise(config, seed=nothing)
+The file that stores all default parameters.
+"""
+const PARAMFILE = "src/parameters.toml"
+## (DO NOT CHANGE THIS VALUE! Instead, specify simulation-specific configuration files
+## by using the "--configfile" commandline argument, or when invoking simulate().) 
+
+"""
+    initialise(config=PARAMFILE, seed=nothing)
 
 Initialise the model: read in parameters, create the output data directory,
-and instantiate the AgentBasedModel object. Optionally allows overriding the
-`seed` parameter.
+and instantiate the AgentBasedModel object. Optionally allows specifying the
+configuration file and overriding the `seed` parameter.
 """
-function initialise(config::String=PARAMFILE, seed::Union{Int64,Nothing)=nothing)
+function initialise(config::String=PARAMFILE, seed::Union{Int64,Nothing}=nothing)
     @info "Simulation run started at $(Dates.now())."
-    #TODO add a seed parameter - requires mutable parameters
-    # do some housekeeping
-    settings = initsettings(config, seed)
+    settings = getsettings(config, seed)
     Random.seed!(settings["core"]["seed"])
-    setupdatadir()
-    # initialise world-level properties
-    landscape = initlandscape()
     events = Vector{FarmEvent}()
+    landscape = initlandscape(settings["core"]["landcovermap"], settings["core"]["farmfieldsmap"])
     space = GridSpace(size(landscape), periodic=false)
     properties = Dict{Symbol,Any}(:settings=>settings,
                                   :date=>settings["core"]["startdate"],
@@ -27,8 +30,8 @@ function initialise(config::String=PARAMFILE, seed::Union{Int64,Nothing)=nothing
                                   :events=>events)
     @debug "Setting up model."
     model = AgentBasedModel(Union{Farmer,Animal,FarmPlot}, space, properties=properties,
-                            rng=Random.Xoshiro(param("core.seed")), warn=false)
-    # initialise submodels
+                            rng=Random.Xoshiro(settings["core"]["seed"]), warn=false)
+    setupdatadir(model)
     initfarms!(model)
     initfields!(model)
     initnature!(model)
@@ -57,20 +60,21 @@ end
 Wrap up the simulation. Currently doesn't do anything except print some information.
 """
 function finalise(model::AgentBasedModel)
-    @info "Simulated $(model.date-param("core.startdate"))."
-    @info "Simulation run completed at $(Dates.now()),\nwrote output to $(param("core.outdir"))."
+    @info "Simulated $(model.date-@param(core.startdate))."
+    @info "Simulation run completed at $(Dates.now()),\nwrote output to $(@param(core.outdir))."
     #XXX is there anything to do here?
     #genocide!(model)
 end
 
 """
-    simulate(config)
+    simulate(config=PARAMFILE, seed=nothing)
 
-Carry out a complete simulation run.
+Carry out a complete simulation run, optionally specifying a configuration file
+and a seed for the RNG.
 """
-function simulate(config::String=PARAMFILE)
-    model = initialise(config) #TODO add seed value
-    runtime = Dates.value(param("core.enddate")-param("core.startdate"))+1
+function simulate(config::String=PARAMFILE, seed::Union{Int64,Nothing}=nothing)
+    model = initialise(config, seed)
+    runtime = Dates.value(@param(core.enddate)-@param(core.startdate))+1
     step!(model, dummystep, stepsimulation!, runtime)
     finalise(model)
 end
diff --git a/src/nature/ecologicaldata.jl b/src/nature/ecologicaldata.jl
index e57b15e..507ad17 100644
--- a/src/nature/ecologicaldata.jl
+++ b/src/nature/ecologicaldata.jl
@@ -11,11 +11,11 @@ const INDFILE = "individuals.csv"
 
 Create output files for each data group collected by the nature model.
 """
-function initecologicaldata()
-    newdataoutput(POPFILE, "Date;Species;Abundance", savepopulationdata,
-                  param("nature.popoutfreq"))
-    newdataoutput(INDFILE, "Date;ID;X;Y;Species;Sex;Age",
-                  saveindividualdata, param("nature.indoutfreq"))
+function initecologicaldata(model::AgentBasedModel)
+    newdataoutput(model, POPFILE, "Date;Species;Abundance", savepopulationdata,
+                  @param(nature.popoutfreq))
+    newdataoutput(model, INDFILE, "Date;ID;X;Y;Species;Sex;Age",
+                  saveindividualdata, @param(nature.indoutfreq))
 end
 
 """
@@ -26,7 +26,7 @@ and population size for each animal species. May be called never, daily, monthly
 yearly, or at the end of a simulation, depending on the parameter `nature.popoutfreq`.
 """
 function savepopulationdata(model::AgentBasedModel)
-    pops = Dict{String,Int}(s=>0 for s = param("nature.targetspecies"))
+    pops = Dict{String,Int}(s=>0 for s = @param(nature.targetspecies))
     for a in allagents(model)
         (typeof(a) != Animal) && continue
         pops[a.traits["name"]] += 1
diff --git a/src/nature/nature.jl b/src/nature/nature.jl
index e15e65b..726d6f7 100644
--- a/src/nature/nature.jl
+++ b/src/nature/nature.jl
@@ -50,12 +50,12 @@ Initialise the model with all simulated animal populations.
 """
 function initnature!(model::AgentBasedModel)
     # The config file determines which species are simulated in this run
-    for speciesname in param("nature.targetspecies")
+    for speciesname in @param(nature.targetspecies)
         species = @eval $(Symbol(speciesname))($model)
         species["initialise!"](species, model)
     end
     # Initialise the data output
-    initecologicaldata()
+    initecologicaldata(model)
 end
 
 
diff --git a/test/io_tests.jl b/test/io_tests.jl
index cb06bed..8534fbe 100644
--- a/test/io_tests.jl
+++ b/test/io_tests.jl
@@ -4,26 +4,31 @@
 ###
 
 @testset "Model configuration" begin
-    # `test_parameters.toml` is read in in `runtests.jl`
-    @test param("core.configfile") == TESTPARAMETERS
-    @test param("core.startdate") == Date(2020, 1, 1)
-    @test param("nature.targetspecies") == ["Wolpertinger", "Wyvern"]
+    properties = Dict{Symbol,Any}(:settings=>TESTSETTINGS)
+    space = GridSpace((10,10), periodic=false)
+    model = AgentBasedModel(Animal, space, properties=properties, warn=false)
+    @test @param(core.configfile) == TESTPARAMETERS
+    @test @param(core.startdate) == Date(2020, 1, 1)
+    @test @param(nature.targetspecies) == ["Wolpertinger", "Wyvern"]
     #TODO test commandline parameters
 end
 
 @testset "Output functions" begin
+    properties = Dict{Symbol,Any}(:settings=>TESTSETTINGS)
+    space = GridSpace((10,10), periodic=false)
+    model = AgentBasedModel(Animal, space, properties=properties, warn=false)
     # test that the output directory is created with all files
     logstring = "Setting up output directory results_testsuite_$(Dates.today())_s1"
-    @test_logs (:debug, logstring) min_level=Logging.Debug Ps.setupdatadir()
-    @test isdir(param("core.outdir"))
-    @test isfile(joinpath(param("core.outdir"), param("core.landcovermap")))
-    @test isfile(joinpath(param("core.outdir"), param("core.farmfieldsmap")))
-    @test isfile(joinpath(param("core.outdir"), param("core.configfile")))
-    @test isfile(joinpath(param("core.outdir"), Ps.LOGFILE))
+    @test_logs (:debug, logstring) min_level=Logging.Debug Ps.setupdatadir(model)
+    @test isdir(@param(core.outdir))
+    @test isfile(joinpath(@param(core.outdir), @param(core.landcovermap)))
+    @test isfile(joinpath(@param(core.outdir), @param(core.farmfieldsmap)))
+    @test isfile(joinpath(@param(core.outdir), @param(core.configfile)))
+    @test isfile(joinpath(@param(core.outdir), Ps.LOGFILE))
     # check whether the overwrite warning/protection works
-    logstring = "Overwriting existing output directory $(param("core.outdir"))."
-    @test_logs (:warn, logstring) match_mode=:any Ps.setupdatadir()
+    logstring = "Overwriting existing output directory $(@param(core.outdir))."
+    @test_logs (:warn, logstring) match_mode=:any Ps.setupdatadir(model)
     #TODO test overwrite protection (requires parameter mutability)
-    rm(param("core.outdir"), force=true, recursive=true)
+    rm(@param(core.outdir), force=true, recursive=true)
     #TODO test that creating a DataOutput works, and outputs data with the required frequency
 end
diff --git a/test/landscape_tests.jl b/test/landscape_tests.jl
index ae6b3b1..78ea08e 100644
--- a/test/landscape_tests.jl
+++ b/test/landscape_tests.jl
@@ -25,17 +25,17 @@ function smalltestlandscape(agenttype::Type=Animal)
     end
     landscape[6,4] = Pixel(Ps.water, 0, [])
     space = GridSpace(size(landscape), periodic=false)
-    properties = Dict{Symbol,Any}(:landscape=>landscape)
+    properties = Dict{Symbol,Any}(:landscape=>landscape, :settings=>TESTSETTINGS)
     return AgentBasedModel(agenttype, space, properties=properties, warn=false)
 end
 
 @testset "Landscape initialisation" begin
     # initialise the landscape part of the model
-    landscape = Ps.initlandscape()
+    landscape = Ps.initlandscape(TESTSETTINGS["core"]["landcovermap"],
+                                 TESTSETTINGS["core"]["farmfieldsmap"])
     space = GridSpace(size(landscape), periodic=false)
-    properties = Dict{Symbol,Any}(:landscape=>landscape)
-    model = AgentBasedModel(FarmPlot, space, properties=properties,
-                            rng=Random.Xoshiro(param("core.seed")), warn=false)
+    properties = Dict{Symbol,Any}(:landscape=>landscape, :settings=>TESTSETTINGS)
+    model = AgentBasedModel(FarmPlot, space, properties=properties, warn=false)
     Ps.initfields!(model)
     # these tests are specific to the Jena maps
     @test size(model.landscape) == (1754, 1602)
@@ -53,13 +53,15 @@ end
 
 @testset "Event system" begin
     # initialise a basic model landscape
-    landscape = Ps.initlandscape()
+    landscape = Ps.initlandscape(TESTSETTINGS["core"]["landcovermap"],
+                                 TESTSETTINGS["core"]["farmfieldsmap"])
     space = GridSpace(size(landscape), periodic=false)
-    properties = Dict{Symbol,Any}(:date=>param("core.startdate"),
+    properties = Dict{Symbol,Any}(:date=>Date(2022, 1, 1),
                                   :landscape=>landscape,
-                                  :events=>Vector{FarmEvent}())
-    model = AgentBasedModel(Union{Farmer,Animal,FarmPlot}, space, properties=properties,
-                            rng=Random.Xoshiro(param("core.seed")), warn=false)
+                                  :events=>Vector{FarmEvent}(),
+                                  :settings=>TESTSETTINGS)
+    model = AgentBasedModel(Union{Farmer,Animal,FarmPlot}, space,
+                            properties=properties, warn=false)
     # create some events and see whether they show up on the map and disappear as they should
     createevent!(model, [(1,1), (1,2), (1,3), (2,1), (2,3)], Ps.tillage)
     createevent!(model, [(1,1), (1,2), (1,3), (2,2)], Ps.sowing, 2)
diff --git a/test/runtests.jl b/test/runtests.jl
index 8ea5976..5d46318 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -16,9 +16,9 @@ using Agents
 const Ps = Persephone
 
 const TESTPARAMETERS = "test_parameters.toml"
+const TESTSETTINGS = Ps.getsettings(TESTPARAMETERS)
 
 @testset "Persephone Tests" begin
-    Ps.initsettings(TESTPARAMETERS)
     @testset "Core model" begin
         include("io_tests.jl")
         include("landscape_tests.jl")
@@ -33,5 +33,5 @@ const TESTPARAMETERS = "test_parameters.toml"
     @testset "Farm model" begin
         include("farm_tests.jl")
     end
-    rm(param("core.outdir"), force=true, recursive=true)
+    rm(TESTSETTINGS["core"]["outdir"], force=true, recursive=true)
 end
-- 
GitLab