Remove ValidPos type (#1093)

---------

Co-authored-by: Adriano Meligrana <[email protected]>

Project.toml

@@ -1,7 +1,7 @@
 name = "Agents"
 uuid = "46ada45e-f475-11e8-01d0-f70cc89e6671"
 authors = ["George Datseris", "Tim DuBois", "Aayush Sabharwal", "Ali Vahdati", "Adriano Meligrana"]
-version = "6.1.9"
+version = "6.1.10"
 
 [deps]
 CSV = "336ed68f-0bac-5ca0-87d4-7b16caf5d00b"

docs/src/devdocs.md

@@ -53,13 +53,13 @@ Creating a new space type within Agents.jl is quite simple and requires the exte
 
 In principle, the following should be done:
 
-1. Think about what the agent position type should be. Add this type to the `ValidPos` union type in `src/core/model_abstract.jl`.
-1. Think about how the space type will keep track of the agent positions, so that it is possible to implement the function [`nearby_ids`](@ref).
-1. Implement the `struct` that represents your new space, while making it a subtype of `AbstractSpace`.
-1. Extend `random_position(model)`.
-1. Extend `add_agent_to_space!(agent, model), remove_agent_from_space!(agent, model)`. This already provides access to `add_agent!, kill_agent!` and `move_agent!`.
-1. Extend `nearby_ids(pos, model, r)`.
-1. Create a new "minimal" agent type to be used with [`@agent`](@ref) (see the source code of [`GraphAgent`](@ref) for an example).
+1. Think about what the agent position type should be.
+2. Think about how the space type will keep track of the agent positions, so that it is possible to implement the function [`nearby_ids`](@ref).
+3. Implement the `struct` that represents your new space, while making it a subtype of `AbstractSpace`.
+4. Extend `random_position(model)`.
+5. Extend `add_agent_to_space!(agent, model), remove_agent_from_space!(agent, model)`. This already provides access to `add_agent!, kill_agent!` and `move_agent!`.
+6. Extend `nearby_ids(pos, model, r)`.
+7. Create a new "minimal" agent type to be used with [`@agent`](@ref) (see the source code of [`GraphAgent`](@ref) for an example).
 
 And that's it! Every function of the main API will now work. In some situations you might want to explicitly extend other functions such as `move_agent!` for performance reasons.
 

src/Agents.jl

@@ -66,6 +66,8 @@ include("deprecations.jl")
 # visualizations (singleton methods for package extension)
 include("visualizations.jl")
 
+include("ambiguities.jl")
+
 include("precompile.jl")
 
 # Update messages:

src/ambiguities.jl

@@ -0,0 +1,7 @@
+
+# non-functional ambiguity fixes
+
+add_agent!(::AbstractAgent, ::Union{Function, Type}, ::AgentBasedModel, ::Vararg{Any, N}; kwargs...) where N = error()
+add_agent!(::AbstractAgent, ::AgentBasedModel, ::AgentBasedModel) = error()
+add_agent!(::AgentBasedModel, ::Union{Function, Type}, ::AgentBasedModel, ::Vararg{Any, N}; kwargs...) where N = error()
+add_agent!(::AgentBasedModel, ::AgentBasedModel, ::Vararg{Any, N}; kwargs...) where N = error()

src/core/model_abstract.jl

@@ -17,15 +17,6 @@ abstract type AbstractSpace end
 abstract type DiscreteSpace <: AbstractSpace end
 SpaceType = Union{Nothing, AbstractSpace}
 
-# This is a collection of valid position types, sometimes used for ambiguity resolution
-ValidPos = Union{
-    Int, # graph
-    NTuple{N,Int}, # grid
-    NTuple{M,<:AbstractFloat}, # continuous
-    SVector{M,<:AbstractFloat}, # continuous
-    Tuple{Int,Int,Float64} # osm
-} where {N,M}
-
 
 """
     AgentBasedModel

src/core/space_interaction_API.jl

@@ -124,7 +124,7 @@ Move agent to the given position, or to a random one if a position is not given.
 
 The agent's position is updated to match `pos` after the move.
 """
-function move_agent!(agent::AbstractAgent, pos::ValidPos, model::ABM)
+function move_agent!(agent::AbstractAgent, pos::Any, model::ABM)
     remove_agent_from_space!(agent, model)
     agent.pos = pos
     add_agent_to_space!(agent, model)
@@ -217,7 +217,7 @@ function add_agent!(agent::AbstractAgent, model::ABM)
     add_agent_own_pos!(agent, model)
 end
 
-function add_agent!(agent::AbstractAgent, pos::ValidPos, model::ABM)
+function add_agent!(agent::AbstractAgent, pos::Any, model::ABM)
     agent.pos = pos
     add_agent_own_pos!(agent, model)
 end
@@ -268,7 +268,7 @@ function add_agent!(A::Union{Function, Type}, model::ABM,
 end
 
 function add_agent!(
-    pos::ValidPos,
+    pos::Any,
     model::ABM,
     args::Vararg{Any, N};
     kwargs...,
@@ -279,7 +279,7 @@ end
 
 # lowest level - actually constructs the agent
 function add_agent!(
-    pos::ValidPos,
+    pos::Any,
     A::Union{Function, Type},
     model::ABM,
     args::Vararg{Any, N};

src/spaces/continuous.jl

@@ -9,7 +9,8 @@ struct ContinuousSpace{D,P,T<:AbstractFloat,F} <: AbstractSpace
     spacing::T
     extent::SVector{D,T}
 end
-Base.eltype(s::ContinuousSpace{D,P,T,F}) where {D,P,T,F} = T
+const ContinuousPos{D,T} = Union{SVector{D,T},NTuple{D,T}} where {T<:AbstractFloat}
+Base.eltype(::ContinuousSpace{D,P,T,F}) where {D,P,T,F} = T
 no_vel_update(a, m) = nothing
 spacesize(space::ContinuousSpace) = space.extent
 function Base.show(io::IO, space::ContinuousSpace{D,P}) where {D,P}
@@ -108,10 +109,10 @@ end
 
 "given position in continuous space, return cell coordinates in grid space."
 pos2cell(a::AbstractAgent, model::ABM) = pos2cell(a.pos, model)
-pos2cell(pos::ValidPos, model::ABM) = Tuple(max.(1, ceil.(Int, pos./abmspace(model).spacing)))
+pos2cell(pos::ContinuousPos, model::ABM) = Tuple(max.(1, ceil.(Int, pos./abmspace(model).spacing)))
 
 "given position in continuous space, return continuous space coordinates of cell center."
-function cell_center(pos::ValidPos, model)
+function cell_center(pos::ContinuousPos, model)
     abmspace(model).spacing .* (pos2cell(pos, model) .- 0.5)
 end
 
@@ -146,7 +147,7 @@ end
 
 # We re-write this for performance, because if cell doesn't change, we don't have to
 # move the agent in the GridSpace; only change its position field
-function move_agent!(agent::AbstractAgent, pos::ValidPos, model::ABM{<:ContinuousSpace})
+function move_agent!(agent::AbstractAgent, pos::ContinuousPos, model::ABM{<:ContinuousSpace})
     space_size = spacesize(model)
     D = length(space_size)
     all(i -> 0 <= pos[i] <= space_size[i], 1:D) || error("position is outside space extent!")
@@ -202,7 +203,7 @@ function offsets_within_radius(model::ABM{<:ContinuousSpace}, r::Real)
     return offsets_within_radius(abmspace(model).grid, r)
 end
 
-function nearby_ids(pos::ValidPos, model::ABM{<:ContinuousSpace}, r = 1; search = :approximate)
+function nearby_ids(pos::ContinuousPos, model::ABM{<:ContinuousSpace}, r = 1; search = :approximate)
     if search === :approximate
         return nearby_ids_approx(pos, model, r)
     elseif search === :exact
@@ -211,7 +212,7 @@ function nearby_ids(pos::ValidPos, model::ABM{<:ContinuousSpace}, r = 1; search
     error("`search` keyword should be either `:approximate` or `:exact`")
 end
 
-function nearby_ids_approx(pos::ValidPos, model::ABM{<:ContinuousSpace}, r = 1)
+function nearby_ids_approx(pos::ContinuousPos, model::ABM{<:ContinuousSpace}, r = 1)
     # Calculate maximum grid distance (distance + distance from cell center)
     δ = distance_from_cell_center(pos, model)
     # Ceiling since we want always to overestimate the radius
@@ -222,7 +223,7 @@ function nearby_ids_approx(pos::ValidPos, model::ABM{<:ContinuousSpace}, r = 1)
     return nearby_ids(focal_cell, abmspace(model).grid, grid_r)
 end
 
-function nearby_ids_exact(pos::ValidPos, model::ABM{<:ContinuousSpace}, r = 1)
+function nearby_ids_exact(pos::ContinuousPos, model::ABM{<:ContinuousSpace}, r = 1)
     # TODO:
     # Simply filtering nearby_ids_approx leads to 4x faster code than the commented-out logic.
     # It is because the code of the "fast logic" is actually super type unstable.

src/spaces/discrete.jl

@@ -78,8 +78,8 @@ end
     isempty(pos, model::ABM{<:DiscreteSpace})
 Return `true` if there are no agents in `position`.
 """
-Base.isempty(pos::ValidPos, model::ABM{<:DiscreteSpace}) = isempty(pos, abmspace(model))
-Base.isempty(pos::ValidPos, space::DiscreteSpace) = isempty(ids_in_position(pos, space))
+Base.isempty(pos::Int, model::ABM{<:DiscreteSpace}) = isempty(pos, abmspace(model))
+Base.isempty(pos::Int, space::DiscreteSpace) = isempty(ids_in_position(pos, space))
 
 """
     has_empty_positions(model::ABM{<:DiscreteSpace})

src/spaces/grid_general.jl

@@ -1,4 +1,4 @@
-export GridAgent
+export GridAgent, GridPos
 
 """
     AbstractGridSpace{D,P}
@@ -10,13 +10,15 @@ indices are the possible positions in the space.
 Furthermore, all spaces should have at least the fields
 * `offsets_within_radius`
 * `offsets_within_radius_no_0`
-which are `Dict{Float64,Vector{NTuple{D,Int}}}`, mapping radii
+which are `Dict{Float64,Vector{GridPos{D}}}`, mapping radii
 to vector of indices within each radius.
 
 `D` is the dimension and `P` is whether the space is periodic (boolean).
 """
 abstract type AbstractGridSpace{D,P} <: DiscreteSpace end
 
+const GridPos{D} = NTuple{D,Int}
+
 """
     GridAgent{D} <: AbstractAgent
 The minimal agent struct for usage with `D`-dimensional [`GridSpace`](@ref).
@@ -176,13 +178,13 @@ end
 # utilizes the above `offsets_within_radius_no_0`. We complicated it a bit more because
 # we want to be able to re-use it in `ContinuousSpace`, so we allow it to either
 # find positions with the 0 or without.
-function nearby_positions(pos::ValidPos, model::ABM{<:AbstractGridSpace}, args::Vararg{Any, N}) where {N}
+function nearby_positions(pos::GridPos{D}, model::ABM{<:AbstractGridSpace{D}}, args::Vararg{Any,N}) where {D,N}
     return nearby_positions(pos, abmspace(model), args...)
 end
 function nearby_positions(
-        pos::ValidPos, space::AbstractGridSpace{D,false}, r = 1,
-        get_indices_f = offsets_within_radius_no_0 # NOT PUBLIC API! For `ContinuousSpace`.
-    ) where {D}
+    pos::GridPos{D}, space::AbstractGridSpace{D,false}, r=1,
+    get_indices_f=offsets_within_radius_no_0 # NOT PUBLIC API! For `ContinuousSpace`.
+) where {D}
     nindices = get_indices_f(space, r)
     space_size = spacesize(space)
     # check if we are far from the wall to skip bounds checks
@@ -194,9 +196,9 @@ function nearby_positions(
     end
 end
 function nearby_positions(
-        pos::ValidPos, space::AbstractGridSpace{D,true}, r = 1,
-        get_indices_f = offsets_within_radius_no_0 # NOT PUBLIC API! For `ContinuousSpace`.
-    ) where {D}
+    pos::GridPos{D}, space::AbstractGridSpace{D,true}, r=1,
+    get_indices_f=offsets_within_radius_no_0 # NOT PUBLIC API! For `ContinuousSpace`.
+) where {D}
     nindices = get_indices_f(space, r)
     space_size = spacesize(space)
     # check if we are far from the wall to skip bounds checks
@@ -209,8 +211,8 @@ function nearby_positions(
     end
 end
 function nearby_positions(
-    pos::ValidPos, space::AbstractGridSpace{D,P}, r = 1,
-    get_indices_f = offsets_within_radius_no_0 # NOT PUBLIC API! For `ContinuousSpace`.
+    pos::GridPos{D}, space::AbstractGridSpace{D,P}, r=1,
+    get_indices_f=offsets_within_radius_no_0 # NOT PUBLIC API! For `ContinuousSpace`.
 ) where {D,P}
     stored_ids = space.stored_ids
     nindices = get_indices_f(space, r)
@@ -228,7 +230,7 @@ function nearby_positions(
     end
 end
 
-function random_nearby_position(pos::ValidPos, model::ABM{<:AbstractGridSpace{D,false}}, r=1; kwargs...) where {D}
+function random_nearby_position(pos::Any, model::ABM{<:AbstractGridSpace{D,false}}, r=1; kwargs...) where {D}
     nindices = offsets_within_radius_no_0(abmspace(model), r)
     stored_ids = abmspace(model).stored_ids
     rng = abmrng(model)
@@ -239,15 +241,15 @@ function random_nearby_position(pos::ValidPos, model::ABM{<:AbstractGridSpace{D,
     end
 end
 
-function random_nearby_position(pos::ValidPos, model::ABM{<:AbstractGridSpace{D,true}}, r=1; kwargs...) where {D}
+function random_nearby_position(pos::GridPos{D}, model::ABM{<:AbstractGridSpace{D,true}}, r=1; kwargs...) where {D}
     nindices = offsets_within_radius_no_0(abmspace(model), r)
     stored_ids = abmspace(model).stored_ids
     chosen_offset = rand(abmrng(model), nindices)
     chosen_pos = pos .+ chosen_offset
     checkbounds(Bool, stored_ids, chosen_pos...) && return chosen_pos
     return mod1.(chosen_pos, spacesize(model))
 end
-  
+
 ###################################################################
 # pretty printing
 ###################################################################

src/spaces/grid_multi.jl

@@ -101,6 +101,9 @@ function remove_agent_from_space!(a::AbstractAgent, model::ABM{<:GridSpace})
     return a
 end
 
+Base.isempty(pos::GridPos, model::ABM{<:GridSpace}) = isempty(pos, abmspace(model))
+Base.isempty(pos::GridPos, space::GridSpace) = isempty(space.stored_ids[pos...])
+
 ##########################################################################################
 # nearby_stuff for GridSpace
 ##########################################################################################
@@ -230,17 +233,17 @@ invalid_access_nocheck(pos_index, iter::GridSpaceIdIterator) = @inbounds isempty
 # TODO: We can re-write this to create its own `indices_within_radius_tuple`.
 # This would also allow it to work for any metric, not just Chebyshev!
 
-function nearby_ids(pos::ValidPos, model::ABM{<:GridSpace}, r::NTuple{D,Int}) where {D}
+function nearby_ids(pos::GridPos{D}, model::ABM{<:GridSpace{D}}, r::NTuple{D,Int}) where {D}
     # simply transform `r` to the Vector format expected by the below function
     newr = [(i, -r[i]:r[i]) for i in 1:D]
     nearby_ids(pos, model, newr)
 end
 
 function nearby_ids(
-    pos::ValidPos,
+    pos::GridPos{D},
     model::ABM{<:GridSpace},
     r::Vector{Tuple{Int64, UnitRange{Int64}}},
-)
+) where D
     @assert abmspace(model).metric == :chebyshev
     dims = first.(r)
     vidx = []
@@ -261,7 +264,7 @@ end
 #######################################################################################
 # %% Further discrete space functions
 #######################################################################################
-ids_in_position(pos::ValidPos, model::ABM{<:GridSpace}) = ids_in_position(pos, abmspace(model))
-function ids_in_position(pos::ValidPos, space::GridSpace)
+ids_in_position(pos::GridPos, model::ABM{<:GridSpace}) = ids_in_position(pos, abmspace(model))
+function ids_in_position(pos::GridPos, space::GridSpace)
     return space.stored_ids[pos...]
 end

src/spaces/grid_single.jl

@@ -58,7 +58,7 @@ end
 # move_agent! does not need be implemented.
 # The generic version at core/space_interaction_API.jl covers it.
 # `random_empty` comes from spaces/discrete.jl as long as we extend:
-Base.isempty(pos::ValidPos, model::ABM{<:GridSpaceSingle}) = abmspace(model).stored_ids[pos...] == 0
+Base.isempty(pos::GridPos{D}, model::ABM{<:GridSpaceSingle{D}}) where D = abmspace(model).stored_ids[pos...] == 0
 # And we also need to extend the iterator of empty positions
 function empty_positions(model::ABM{<:GridSpaceSingle})
     Iterators.filter(i -> abmspace(model).stored_ids[i...] == 0, positions(model))
@@ -73,7 +73,7 @@ This will be `0` if there is no agent in this position.
 This is similar to [`ids_in_position`](@ref), but specialized for `GridSpaceSingle`.
 See also [`isempty`](@ref).
 """
-function id_in_position(pos, model::ABM{<:GridSpaceSingle})
+function id_in_position(pos::GridPos, model::ABM{<:GridSpaceSingle})
     return abmspace(model).stored_ids[pos...]
 end