Addressed the remaining comments from the first code review

closes #40

README.md

@@ -9,8 +9,8 @@
 [Persefone.jl](https://persefone-model.eu/) models agricultural practice and 
 how it impacts animal species at a landscape scale. It includes a farm submodel, a crop 
 growth submodel, and individual-based models of multiple indicator species. Its aim is to 
-investigate how changes in farm operations (e.g. through policy changes in the CAP) 
-influence biodiversity.
+investigate how changes in farm operations (e.g. through changes in the European Common
+Agricultural Policy) influence biodiversity.
 
 **The model is still in development. A first version will be available in summer 2024.**
 
@@ -22,9 +22,9 @@ To use Persefone.jl with a graphical user interface, see [here](https://git.idiv
 ### Installation
 
 Install the latest version of the [Julia](https://julialang.org/downloads/) programming 
-language (1.9+). The recommended editors are [VSCode](https://www.julia-vscode.org/) or
+language (1.10+). The recommended editors are [VSCode](https://www.julia-vscode.org/) or
 [Emacs](https://www.emacswiki.org/emacs/JuliaProgrammingLanguage).
-To install package dependencies, open a Julia REPL in this folder and run 
+To install package dependencies, open a Julia REPL in the Persefone root folder and run 
 `using Pkg; Pkg.activate("."); Pkg.instantiate()`.
 
 ### Running from the command line
@@ -81,8 +81,12 @@ Pkg.activate(".") # assuming you're in the Persefone root folder
 using Persefone
 ```
 
-You can then access all Persefone functions, such as `simulate()`. (See
-`src/Persefone.jl` for a list of exported functions.)
+You can then access all Persefone functions, such as 
+[`simulate()`](https://persefone-model.eu/documentation/simulation.html#Persefone.simulate) 
+(which runs a complete simulation, as when calling `julia run.jl` from the commandline). 
+See [`src/Persefone.jl`](https://git.idiv.de/persefone/persefone-model/-/blob/master/src/Persefone.jl?ref_type=heads) 
+or the [documentation](https://persefone-model.eu/documentation/simulation.html) for a 
+list of exported functions.
 
 ---
 

src/Persefone.jl

@@ -22,7 +22,6 @@ using
     DataFramesMeta,
     Distributed,
     FileIO,
-    #FIXME an upstream update broke GeoArrays for TableTransforms > 1.15.0
     GeoArrays, #XXX this is another big dependency
     Logging,
     LoggingExtras,

src/core/input.jl

@@ -4,7 +4,8 @@
 ###
 
 ## Note: much of this code was adapted from the GeMM model by Leidinger et al.
-## (https://github.com/CCTB-Ecomods/gemm/blob/master/src/input.jl)
+## (https://github.com/CCTB-Ecomods/gemm/blob/master/src/input.jl, archived at
+## https://doi.org/10.5281/zenodo.5602906)
 
 """
 The file that stores all default parameters: `src/parameters.toml`
@@ -75,8 +76,8 @@ end
 """
     preprocessparameters(settings)
 
-Take the raw input parameters and process them (convert types, perform checks, etc.).
-This is a helper function for [`getsettings`](@ref).
+Take the raw input parameters and process them where necessary (e.g. convert types or
+perform checks). This is a helper function for [`getsettings`](@ref).
 """
 function preprocessparameters(settings::Dict{String,Any}, defaultoutdir::String)
     (settings["core.seed"] == 0) && (settings["core.seed"] = abs(rand(RandomDevice(), Int32)))

src/core/output.jl

 ### Persefone.jl - a model of agricultural landscapes and ecosystems in Europe.
 ###
-### This file includes functions for saving the model output.
+### This file contains functions for saving the model output. This includes logging
+### facilities, but also functions for collecting and outputting data from the
+### different submodels.
 ###
 
+"Log output is saved to `simulation.log` in the output directory"
 const LOGFILE = "simulation.log"
 
+"All input data are copied to the `inputs` folder within the output directory"
+const RECORDDIR = "inputs"
+
 ## Much of this code was adapted from the GeMM model by Leidinger et al.
 ## (https://github.com/CCTB-Ecomods/gemm/blob/master/src/output.jl)
 
@@ -26,6 +32,7 @@ function createdatadir(outdir::String, overwrite::Union{Bool,String})
             @warn "Overwriting existing output directory $(outdir)."
         #TODO replace with exception
     end
+    @debug "Setting up output directory $outdir."
     mkpath(outdir)
 end
 
@@ -81,9 +88,9 @@ settings used. This allows replicating a run in future.
 function saveinputfiles(model::SimulationModel)
     #XXX If this is a parallel run, we should save the global config to the top-level
     # output directory
-    @debug "Setting up output directory $(@param(core.outdir))."
     currentcommit = read(`git rev-parse HEAD`, String)[1:8]
-    open(joinpath(@param(core.outdir), basename(@param(core.configfile))), "w") do f
+    mkpath(joinpath(@param(core.outdir), RECORDDIR))
+    open(joinpath(@param(core.outdir), RECORDDIR, basename(@param(core.configfile))), "w") do f
         println(f, "#\n# --- Persefone 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"))),")
@@ -96,14 +103,17 @@ function saveinputfiles(model::SimulationModel)
         end
         TOML.print(f, prepareTOML(deepcopy(model.settings)))
     end
-    # Copy the map files to the output folder
+    # Copy the map and weather files to the output folder
     lcmap = joinpath(@param(world.mapdirectory), @param(world.landcovermap))
     ffmap = joinpath(@param(world.mapdirectory), @param(world.farmfieldsmap))
+    wfile = joinpath(@param(world.mapdirectory), @param(world.weatherfile))
     #TODO replace errors with exceptions
     !(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)
+    !(isfile(wfile)) && Base.error("The map file $(wfile) doesn't exist.")
+    cp(lcmap, joinpath(@param(core.outdir), RECORDDIR, basename(lcmap)), force = true)
+    cp(ffmap, joinpath(@param(core.outdir), RECORDDIR, basename(ffmap)), force = true)
+    cp(wfile, joinpath(@param(core.outdir), RECORDDIR, basename(wfile)), force = true)
 end
 
 """

src/nature/ecologicaldata.jl

 ### Persefone.jl - a model of agricultural landscapes and ecosystems in Europe.
 ###
-### This file includes the functions for collecting and saving ecological output data.
+### This file includes the functions for collecting ecological output data, which
+### are then passed on to the `core/output.jl`.
 ###
 
 """
@@ -18,9 +19,9 @@ end
 """
     savepopulationdata(model)
 
-Return a comma-separated set of lines (to be printed to `populations.csv`), giving
-the current date 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
+Return a data table (to be printed to `populations.csv`), giving the current
+date 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::SimulationModel)
@@ -42,8 +43,8 @@ end
 """
     saveindividualdata(model)
 
-Return a comma-separated set of lines (to be printed to `individuals.csv`), listing
-all properties of all animal individuals in the model. May be called never, daily,
+Return a data table (to be printed to `individuals.csv`), listing all
+properties of all animal individuals in the model. May be called never, daily,
 monthly, yearly, or at the end of a simulation, depending on the parameter
 `nature.indoutfreq`. WARNING: Produces very big files!
 """

src/nature/energy.jl

@@ -3,6 +3,7 @@
 ### This file contains structs and functions for implementing Dynamic Energy Budgets.
 ###
 
+#TODO add units
 
 ## STRUCTS
 
@@ -42,7 +43,7 @@ in a reserve buffer, before being used for maintenance, growth, and reproduction
 - Sibly et al. (2013). Representing the acquisition and use of energy by individuals in agent-based models of animal populations. Methods in Ecology and Evolution, 4(2), 151–161. https://doi.org/10.1111/2041-210x.12002
 - Sousa et al. (2010). Dynamic energy budget theory restores coherence in biology. Philosophical Transactions of the Royal Society B: Biological Sciences, 365(1557), 3413–3428. https://doi.org/10.1098/rstb.2010.0166
 - Kooijman, S. A. L. M. (2009). Dynamic energy and mass budgets in biological systems (3rd ed). Cambridge University Press. https://www.researchgate.net/profile/Edgar-Meza-3/post/Is_there_a_toxicokinetic_model_for_daphnia_magna_or_other_zooplankton/attachment/59d62cf579197b807798b396/AS%3A348547653357569%401460111644286/download/Dynamic+Energy+Budget+theory+-+Kooijman.pdf
-- *see also:* Brown et al. (2004). Toward a metabolic theory of ecology. Ecology, 85(7), 1771–1789. https://doi.org/10.1890/03-9000
+- *compare with:* Brown et al. (2004). Toward a metabolic theory of ecology. Ecology, 85(7), 1771–1789. https://doi.org/10.1890/03-9000
 """
 mutable struct EnergyBudget
     params::DEBparameters

src/nature/populations.jl

@@ -70,10 +70,11 @@ end
     initpopulation!(speciestype, popinitparams, model)
 
 Initialise the population of the given species, based on the given initialisation parameters.
-This is an internal function called by initpopulation!(), and was split off from it to allow
+This is an internal function called by `initpopulation!()`, and was split off from it to allow
 better testing.
 """
-function initpopulation!(species::Type, p::PopInitParams, model::SimulationModel)   
+function initpopulation!(species::Type, p::PopInitParams, model::SimulationModel)
+    #XXX this is a pretty complicated function - can we make it simpler?
     (p.popsize <= 0 && p.indarea <= 0m²) && # can be legit if a habitat descriptor is provided
         @warn("initpopulation!() called with popsize and indarea both <= 0")
     (p.popsize > 0 && p.indarea > 0m²) && #XXX not sure what this would do
@@ -93,22 +94,7 @@ function initpopulation!(species::Type, p::PopInitParams, model::SimulationModel
                     #XXX `n==0` above guarantees that at least one individual is created, even
                     # in a landscape that is otherwise too small for the specified indarea -
                     # do we want this?
-                    if p.pairs
-                        a1 = species(length(model.animals)+1, male, (-1, -1), (x,y), p.initphase)
-                        a2 = species(length(model.animals)+2, female, (-1, -1), (x,y), p.initphase)
-                        push!(model.animals, a1, a2)
-                        push!(model.landscape[x,y].animals, a1.id, a2.id)
-                        create!(a1, model)
-                        create!(a2, model)
-                        n += 2
-                    else
-                        sex = p.asexual ? hermaphrodite : @rand([male, female])
-                        a = species(length(model.animals)+1, sex, (-1, -1), (x,y), p.initphase)
-                        push!(model.animals, a)
-                        push!(model.landscape[x,y].animals, a.id)
-                        create!(a, model)
-                        n += 1
-                    end
+                    n += initindividuals!(species, (x,y), p, model)
                 end
                 #XXX break randomly to avoid initialising all individuals in a single column?
                 (p.popsize > 0 && n >= p.popsize) && break
@@ -124,6 +110,32 @@ function initpopulation!(species::Type, p::PopInitParams, model::SimulationModel
     @info "Initialised $(n) $(speciesof(species))s."
 end
 
+"""
+    initindividuals!(species, pos, popinitparams, model)
+
+Initialise one or two individuals (depending on the `pairs` parameter) in the
+given location. Returns the number of created individuals. (Internal helper
+function for `initpopulation!()`.)
+"""
+function initindividuals!(species::Type, pos::Tuple{Int64,Int64}, p::PopInitParams, model::SimulationModel)
+    if p.pairs
+        a1 = species(length(model.animals)+1, male, (-1, -1), pos, p.initphase)
+        a2 = species(length(model.animals)+2, female, (-1, -1), pos, p.initphase)
+        push!(model.animals, a1, a2)
+        push!(model.landscape[pos...].animals, a1.id, a2.id)
+        create!(a1, model)
+        create!(a2, model)
+        return 2
+    else
+        sex = p.asexual ? hermaphrodite : @rand([male, female])
+        a = species(length(model.animals)+1, sex, (-1, -1), pos, p.initphase)
+        push!(model.animals, a)
+        push!(model.landscape[pos...].animals, a.id)
+        create!(a, model)
+        return 1
+    end
+end
+
 #XXX initpopulation with dispersal from an original source?
 #XXX initpopulation based on known occurences in real-life?
 
@@ -143,6 +155,7 @@ function reproduce!(animal::Animal, model::SimulationModel,
             sex = @rand([male, female])
         end
         bphase = populationparameters(typeof(animal)).birthphase
+        #TODO add DEB?
         child = typeof(animal)(length(model.animals)+1, sex, (animal.id, mate), animal.pos, bphase)
         push!(model.animals, child)
         push!(animal.offspring, child.id)
@@ -219,7 +232,6 @@ function nearby_ids(pos::Tuple{Int64,Int64}, model::SimulationModel, radius::Len
     ids
 end
 
-
 """
     nearby_animals(pos, model; radius= 0, species="")
 

test/io_tests.jl

@@ -17,14 +17,15 @@ end
     model = inittestmodel()
     # test that the output directory is created with all files
     outdir = @param(core.outdir)
-    Ps.createdatadir(outdir, @param(core.overwrite))
-    @test isdir(outdir)
     @test_logs((:debug, "Setting up output directory results_testsuite."),
                min_level=Logging.Debug, match_mode=:any,
-               Ps.saveinputfiles(model))
-    @test isfile(joinpath(outdir, @param(world.landcovermap)))
-    @test isfile(joinpath(outdir, @param(world.farmfieldsmap)))
-    @test isfile(joinpath(outdir, @param(core.configfile)))
+               Ps.createdatadir(outdir, @param(core.overwrite)))
+    @test isdir(outdir)
+    Ps.saveinputfiles(model)
+    @test isfile(joinpath(outdir, Ps.RECORDDIR, @param(world.landcovermap)))
+    @test isfile(joinpath(outdir, Ps.RECORDDIR, @param(world.farmfieldsmap)))
+    @test isfile(joinpath(outdir, Ps.RECORDDIR, @param(world.weatherfile)))
+    @test isfile(joinpath(outdir, Ps.RECORDDIR, @param(core.configfile)))
     # test log output to screen/file/both
     #XXX cannot test logger output due to https://github.com/JuliaLang/julia/issues/48456
     logger1 = Ps.modellogger(@param(core.loglevel), outdir, "screen")