Closed mastrof closed 1 year ago
I guess this is a very fringe case if it never came up but I have a set of fairly large simulations that simply cannot be run due to this approximation error.
Good catch.
What about changing
all(i -> 0 <= pos[i] < space_size[i], 1:D) || error("position is outside space extent!")
in
all(i -> 0 <= pos[i] <= space_size[i], 1:D) || error("position is outside space extent!")
does this have any unwanted side effect?
edit: yes, move_agent!
errors since the pos2cell
conversion goes outside the grid extent
But changing also
pos2cell(pos::ValidPos, model::ABM) = Tuple(floor.(Int, pos./abmspace(model).spacing) .+ 1)
to
pos2cell(pos::ValidPos, model::ABM) = Tuple(ceil.(Int, pos./abmspace(model).spacing))
it seems to work fine.
The problem is that this is slightly breaking since for an integer position inside the grid, this will change the result of the position of the agent in the grid
Actually it is not breaking since the grid is an internal field only used to improve efficiency of neighbors searches. So, is this fine?
I tried to implement your approach but it breaks a bunch of things, throwing errors of the form
BoundsError: attempt to access 20×20 Matrix{Vector{Int64}} at index [0, 0]
I guess the problem is because 0 is a valid position in ContinuousSpace. So instead of bumping it up to index 1, ceil(0)
returns 0, which is not good for a GridSpace.
In principle it could be solved by always having a call like normalize_position(pos, abmspace(model).grid)
before doing anything with the position returned by pos2cell
so it is brought back to a valid grid position, but here I think it gets very tricky because we still want invalid position to throw errors, not being re-mapped into a valid one.
E.g. with your correction, you cannot even create an agent at (0,0)
because the code cannot place it on the grid; so you would have to add a normalize_position
already at creation, but I'm not sure if that would also allow some invalid positions to slip through.
you are right, then I guess we should just handle the special case with a branch which checks if the value is 0 and makes it 1, hopefully branch prediction should anyway make the code efficient. Otherwise we could clamp it before doing the ceiling
an even simpler way is to use max between 1 and the ceiling
move_agent!
andwalk!
throwposition is outside of space extent
error, apparently due to floating point approximationMinimal Working Example
This particular MWE is on main, but I originally encountered the error on a code running 5.14.0.
What happens is the following (let
agent = model[1]
): the direction of motion is evaluated asthen the "raw" target position would be
which, when passed to
normalize_position
, becomeswhich errors since 1.0 is not a valid position