Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mastrof opened this issue Aug 22, 2023 · 3 comments · Fixed by #856
Closed

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

mastrof opened this issue Aug 22, 2023 · 3 comments · Fixed by #856

Comments

@mastrof
Copy link
Contributor

mastrof commented Aug 22, 2023

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
Copy link
Contributor Author

mastrof commented Aug 22, 2023

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
Copy link
Member

Tortar commented Aug 22, 2023

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
Copy link
Contributor Author

mastrof commented Aug 22, 2023

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})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants