nearby_ids returns the same ID multiple times when r is bigger than the space size

[jarbus]

Describe the bug
nearby_ids returns the same ID multiple times

Minimal Working Example
Modified from flock.jl

using Agents
mutable struct Bird <: AbstractAgent
    id::Int
    pos::NTuple{2,Float64}
end

space2d = ContinuousSpace((100,100),5.0)
model = ABM(Bird, space2d)
for _ in 1:3
    add_agent!( model)
end

println(collect(nearby_ids(model.agents[2], model, 100; exact=false)))

julia> include("flock2.jl")
[1, 3, 3, 1, 3]
AgentBasedModel with 3 agents of type Bird
 space: periodic continuous space with 30×30 divisions
 scheduler: randomly

Agents.jl version
4.5.6

[jarbus]

This seems to be oriented around model.space.grid.s having lots of duplicate ids in a single vector. What exactly is model.space.grid.s for? Is it a Vector{Vector{Int}} where each Vector{Int} is the list of ids in a section of the space?

[AayushSabharwal]

This is interesting. Your interpretation of model.space.grid.s is correct. I'm not entirely sure why this happens, but it's definitely something to look into. I'll try running this on my machine in a while and see what happens.

[AayushSabharwal]

I get a similarly incorrect output. model.space.grid.s as shown here seems to be fine, so it's likely an issue with nearby_ids

[Datseris]

This is a problem with periodic = true. If I run exactly the same thing with periodic = false I get the correct answer. I think we need an extra clause to not count periodically found neighbors if they are already found in a non-periodic manner. In fact, this is a problem of DiscreteSpace, not ContinuousSpace. Same exact thing would happen in discrete space.

To be fair, I'm not sure whether we should do this... Adding an extra if clause increases compute time. But, how often do you really need nearby_agents to search with a radious so large that it covers all of space? Because they only scenario where a neighbor is both periodically nearby, but also non periodically, is when the search radius is literally all of space...

[Datseris]

Yeah, this is non-fixable:

function _grid_space_neighborhood(
    α::CartesianIndex,
    space::GridSpace{D,true},
    hood,
) where {D}
    return ((mod1.(Tuple(α + β), hood.whole.maxi)) for β in hood.βs)
end

we will lose some crazy amount of optimization if we try and fix this issue, and in my eyes it is just not worth it.

[jarbus]

@Datseris Not sure if you are already on top of this, but you should probably update the documentation for this function to let users know about this quirk and an explanation of why it still exists, otherwise you'll be getting a lot more issues/PRs like this one.

[Datseris]

I'm working on something else at the moment, can you open a PR?

[jarbus]

PR created @Datseris

[Tortar]

this is easily fixable actually at least now, just filter hypercube = Iterators.product(repeat([-r:r], D)...) in calculate_offsets such that it doesn't wraparound too much, I think this should be done since it has no perf penalty (since calculate_offsets is calculated only one time for each value of radius)

[Tortar]

sorry to reopen this old issue, actually it is a bit more difficult than this, so I think not worth it

[Tortar]

sorry again it is actually fixable with what I had in mind, double confused myself :D