JuliaDynamics / Agents.jl

Agent-based modeling framework in Julia
https://juliadynamics.github.io/Agents.jl/stable/
MIT License
725 stars 117 forks source link

Unnoticed breaking change in #846: broken model creation with `ContinuousAgent{D}` #855

Closed mastrof closed 1 year ago

mastrof commented 1 year ago

On main this now errors:

julia> ABM(ContinuousAgent{2}, space)
┌ Warning: Agent type is not concrete. If your agent is parametrically typed, you're probably
│ seeing this warning because you gave `Agent` instead of `Agent{Float64}`
│ (for example) to this function. You can also create an instance of your agent
│ and pass it to this function. If you want to use `Union` types for mixed agent
│ models, you can silence this warning.
└ @ Agents ~/.julia/dev/Agents/src/core/model_concrete.jl:139
ERROR: ArgumentError: `pos` field in agent type must be of the same type of the `extent` field in ContinuousSpace.
Stacktrace:
 [1] do_checks(#unused#::Type{ContinuousAgent{2}}, space::ContinuousSpace{2, true, Float64, typeof(Agents.no_vel_update)}, warn::Bool)
   @ Agents ~/.julia/dev/Agents/src/core/model_concrete.jl:191
 [2] agent_validator(#unused#::Type{ContinuousAgent{2}}, space::ContinuousSpace{2, true, Float64, typeof(Agents.no_vel_update)}, warn::Bool)
   @ Agents ~/.julia/dev/Agents/src/core/model_concrete.jl:147
 [3] Agents.SingleContainerABM(::Type{ContinuousAgent{2}}, space::ContinuousSpace{2, true, Float64, typeof(Agents.no_vel_update)}; container::Type, scheduler::typeof(Agents.Schedulers.fastest), properties::Nothing, rng::Random.TaskLocalRNG, warn::Bool)
   @ Agents ~/.julia/dev/Agents/src/core/model_concrete.jl:47
 [4] Agents.SingleContainerABM(::Type{ContinuousAgent{2}}, space::ContinuousSpace{2, true, Float64, typeof(Agents.no_vel_update)})
   @ Agents ~/.julia/dev/Agents/src/core/model_concrete.jl:38
 [5] AgentBasedModel(::Type, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ Agents ~/.julia/dev/Agents/src/deprecations.jl:27
 [6] AgentBasedModel(::Type, ::Vararg{Any})
   @ Agents ~/.julia/dev/Agents/src/deprecations.jl:27
 [7] top-level scope
   @ REPL[24]:1

We had agreed to never use ContinuousAgent{D} without specifying the second type T, but with the back and forth between different versions and the additional checks on the space type at some point it has slipped through and seems we have broken compatibility for model creation. Now since ContinuousAgent{2} is not concrete, we fail the check against the space extent.

I guess we should just add an exception to the do_checks function

mastrof commented 1 year ago

This problem was mentioned at some point in #846 but maybe it flew past us: even if now I add an exception for the do_checks, which is no big deal, any ABM defined for ContinuousAgent{D} will not be concretely typed (while anything inheriting from ContinuousAgent{D} will be fine). Do we just highlight this in the update message so people know to use ContinuousAgent{D,Float64} instead?

Tortar commented 1 year ago

yes, we need to revision the do_checks in my opinion, so that it doesn't error. I think that indeed at some point this didn't error in the PR

mastrof commented 1 year ago

The agents you create with ContinuousAgent{D}(...) are concretely typed; but the ABM does not know it in advance. After a local edit to change the checks (will open a PR soon):


julia> ABM(ContinuousAgent{2}, ContinuousSpace((1,1)); warn=false) |> typeof
StandardABM{ContinuousSpace{2, true, Float64, typeof(no_vel_update)}, ContinuousAgent{2}, typeof(fastest), Nothing, TaskLocalRNG} (alias for Agents.SingleContainerABM{ContinuousSpace{2, true, Float64, typeof(Agents.no_vel_update)}, ContinuousAgent{2}, Dict{Int64, ContinuousAgent{2}}, typeof(Agents.Schedulers.fastest), Nothing, Random.TaskLocalRNG})

julia> ABM(ContinuousAgent{2,Float64}, ContinuousSpace((1,1))) |> typeof
StandardABM{ContinuousSpace{2, true, Float64, typeof(no_vel_update)}, ContinuousAgent{2, Float64}, typeof(fastest), Nothing, TaskLocalRNG} (alias for Agents.SingleContainerABM{ContinuousSpace{2, true, Float64, typeof(Agents.no_vel_update)}, ContinuousAgent{2, Float64}, Dict{Int64, ContinuousAgent{2, Float64}}, typeof(Agents.Schedulers.fastest), Nothing, Random.TaskLocalRNG})