From f7d339d578ab048e7298396464621efd18f0e40d Mon Sep 17 00:00:00 2001
From: Daniel Vedder <daniel.vedder@idiv.de>
Date: Fri, 20 Jan 2023 10:49:17 +0100
Subject: [PATCH] Bugfixes in the species macros

---
 src/nature/nature.jl      | 34 ++++++++++++++++++++--------------
 src/nature/populations.jl |  7 ++-----
 test/nature_tests.jl      | 15 ++++++---------
 3 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/src/nature/nature.jl b/src/nature/nature.jl
index 27dc09b..c8696da 100644
--- a/src/nature/nature.jl
+++ b/src/nature/nature.jl
@@ -40,7 +40,6 @@ end
 Update an animal by one day, executing it's currently active phase function.
 """
 function stepagent!(animal::Animal, model::AgentBasedModel)
-    @debug "Updating $(animalid(animal))."
     animal.age += 1
     animal.traits[animal.traits["phase"]](animal,model)
 end
@@ -73,7 +72,7 @@ custom syntax to describe species' biology:
 ```julia
 @species name begin
 
-    @initialise!(@habitat(...))
+    @initialise(@habitat(...))
     speciesvar1 = 3.14
     ...
 
@@ -96,22 +95,28 @@ the `model` variable (an object of type `AgentBasedModel`).
 macro species(name, body)
     quote
         Core.@__doc__ function $(esc(name))($(esc(:model))::AgentBasedModel)
+            # create internal variables and execute the definition body
             $(esc(:name)) = string($(QuoteNode(name)))
             $(esc(:phase)) = ""
             $(esc(body))
+            # extract and process the local variables into a species dict
             vardict = Base.@locals
             speciesdict = Dict{String,Any}()
-            #delete!(speciesdict, $name) #FIXME remove circular reference from speciesdict
             for k in keys(vardict)
                 speciesdict[string(k)] = vardict[k]
             end
+            delete!(speciesdict, "model")
+            delete!(speciesdict, $(string(name)))
             return speciesdict
         end
+        # allow species to be defined outside of the Persephone module, but still available
+        # inside it (needed by `initnature!()` and `reproduce!()`)
+        (@__MODULE__() != $(esc(:Persephone))) && ($(esc(:Persephone)).$name = $(esc(name)))
     end
 end
 
 """
-    @initialise!(habitatdescriptor; kwargs...)
+    @initialise(habitatdescriptor; kwargs...)
 
 Call this macro within the body of `@species`. It passes the given habitat descriptor
 function and keyword arguments on to `initpopulation()` when setting up the simulation.
@@ -119,7 +124,7 @@ function and keyword arguments on to `initpopulation()` when setting up the simu
 Note: if this macro is not used, the variable `initialise!` must be set manually in the
 species definition.
 """
-macro initialise!(habitatdescriptor, kwargs...)
+macro initialise(habitatdescriptor, kwargs...)
     #FIXME does the habitatdescriptor need to be `esc`aped due to scoping issues?
     :($(esc(:initialise!)) = initpopulation($habitatdescriptor, $(kwargs...)))
 end
@@ -154,10 +159,8 @@ the phase that animals are assigned at birth, unless the variable `phase` is
 explicitly defined by the user in the species definition block.
 """
 macro phase(name, body)
-    #XXX make this documentable?
-    #FIXME Somehow, errors in the phase body are not shown?
     quote
-        Core.@__doc__ $(esc(name)) = function($(esc(:animal))::Animal, $(esc(:model))::AgentBasedModel)
+        Core.@__doc__ function $(esc(name))($(esc(:animal))::Animal, $(esc(:model))::AgentBasedModel)
             $(esc(:pos)) = $(esc(:animal)).pos
             $(esc(body))
         end
@@ -209,22 +212,23 @@ end
 """
     @here(property)
 
-A utility macro to quickly access a property of the animal's current position.
+A utility macro to quickly access a property of the animal's current position
+(i.e. `landcover`, `fieldid`, or  `events` - see the `Pixel` struct).
 This can only be used nested within `@phase`.
 """
 macro here(property)
-    :(model.landscape[animal.pos...].$(property))
+    :($(esc(:model)).landscape[$(esc(:animal)).pos...].$(property))
 end
 
 """
     @kill
 
-Kill this animal. This is a thin wrapper around `kill!()`, and passes on any arguments.
-This can only be used nested within `@phase`.
+Kill this animal (and immediately abort its current update). This is a thin wrapper
+around `kill!()`, and passes on any arguments. This can only be used nested within `@phase`.
 """
 macro kill(args...)
     quote
-        kill!($(esc(:animal)), $(esc(:model)), $(args...))
+        kill!($(esc(:animal)), $(esc(:model)), $(map(esc, args)...))
         return
     end
 end
@@ -236,7 +240,7 @@ Let this animal reproduce. This is a thin wrapper around `reproduce!()`, and pas
 any arguments. This can only be used nested within `@phase`.
 """
 macro reproduce(args...)
-    :(reproduce!($(esc(:animal)), $(esc(:model)), $(args...)))
+    :(reproduce!($(esc(:animal)), $(esc(:model)), $(map(esc, args)...)))
 end
 
 """
@@ -279,6 +283,8 @@ For more complex habitat suitability checks, the use of this macro can be
 circumvented by directly creating an equivalent function.
 """
 macro habitat(body)
+    #XXX I suspect that I may have less problems with macro expansion and module
+    # scoping if @habitat did not create a new function. But is there a different way?
     quote
         function($(esc(:pos)), $(esc(:model)))
             if $(esc(body))
diff --git a/src/nature/populations.jl b/src/nature/populations.jl
index f4e05f7..dbe6924 100644
--- a/src/nature/populations.jl
+++ b/src/nature/populations.jl
@@ -43,10 +43,6 @@ function initpopulation(habitatdescriptor::Function; phase::Union{String,Nothing
             for x in shuffle!(Vector(1:width))
                 for y in shuffle!(Vector(1:height))
                     if habitatdescriptor((x,y), model)
-                        #FIXME we probably have to do a deepcopy here, to prevent all conspecifics
-                        # sharing the same speciesdict object. However, this is (a) expensive
-                        # and (b) we seem to have a self-reference in the speciesdict, leading to
-                        # an infinity loop on deepcopying...
                         if pairs
                             add_agent!((x,y), Animal, model, deepcopy(species), female, 0)
                             add_agent!((x,y), Animal, model, deepcopy(species), male, 0)
@@ -93,7 +89,8 @@ Produce one or more offspring for the given animal at its current location.
 function reproduce!(animal::Animal, model::AgentBasedModel, n::Int64=1)
     for i in 1:n
         sex = (animal.sex == hermaphrodite) ? hermaphrodite : rand([male, female])
-        species = animal.traits # @eval $(Symbol(animal.traits["name"]))($model) #FIXME
+        # We need to generate a fresh species dict here
+        species = @eval $(Symbol(animal.traits["name"]))($model)
         add_agent!(animal.pos, Animal, model, species, sex, 0)
     end
     @debug "$(animalid(animal)) has reproduced."
diff --git a/test/nature_tests.jl b/test/nature_tests.jl
index 74b9c15..e44fbc0 100644
--- a/test/nature_tests.jl
+++ b/test/nature_tests.jl
@@ -67,37 +67,34 @@ end
 end
 
 @testset "Species macros" begin
-    # set up an example landscape and species
-    # (note: have to use `Persephone` instead of `Ps` due to macro expansion issues)
     #FIXME I am having tons of trouble here with module scoping and macro expansion,
     # a known tricky issue in Julia (https://github.com/JuliaLang/julia/issues/23221,
     # https://github.com/p-i-/MetaGuideJulia/wiki#example-swap-macro-to-illustrate-esc).
     # Basically, one needs to avoid nesting modules too deeply (i.e. more than one nesting level)
+    # -- is that so?? --
     model = smalltestlandscape(Union{Animal,Farmer,FarmPlot})
-    
+
     @species Mermaid begin
         ageofmaturity = 2
+        pesticidemortality = 1.0
         #@initialise!(@habitat(@landcover() == Persephone.water), pairs=true) #FIXME
         initialise! = Persephone.initpopulation(@habitat(@landcover() == Persephone.water), pairs=true)
-
         @phase life begin
             @debug "$(Persephone.animalid(animal)) is swimming happily in its pond."
-            @respond Persephone.pesticide @kill()
+            @respond Persephone.pesticide @kill(@trait(pesticidemortality))
             @respond Persephone.harvest @setphase(drought)
             @debug "Animal: $animal"
             if @trait(sex) == Persephone.female && @countanimals() < 3 &&
-                @trait(age) >= @trait(ageofmaturity)
-                @debug "$(Persephone.animalid(animal)) is reproducing."
+                @trait(age) >= @trait(ageofmaturity) && @here(landcover) == Persephone.water
                 @reproduce()
             end
         end
-
         @phase drought begin
             @debug "$(Persephone.animalid(animal)) is trying to survive a drought."
             @respond Persephone.sowing @setphase(life)
         end
     end
-    
+
     # test a complete mermaid life cycle
     pond = (6,4)
     mermaid = Mermaid(model)
-- 
GitLab