-
Notifications
You must be signed in to change notification settings - Fork 925
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
Clarify ContinuousSpace.get_neighbors behavior with multiple agents at same position #2599
Conversation
Performance benchmarks:
|
Sounds like it’s working like intended, right? All agents with the same location are not considered neighbors. Specifically for flocking, since agents are on continuous coordinates, isn’t the chance extremely small a neighbor has the exact same location (fp32 precision?) as another agent? |
I beg to differ. To me, if a given agent X asks for its neighbors, I expect to get all neighbors even if there happens to be an agent Y on the exact same location this agent Y is still a neighbor of agent X.
Yes, this is very much an edge-case kind of issue, which is why the documentation solution is fine as far as I am concerned. |
for more information, see https://pre-commit.ci
Co-authored-by: Ewout ter Hoeven <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
I updated the PR title and body to reflect the final changes. Feel free to adjust further or let me know if you don't like it. |
This PR clarifies the behavior of
ContinuousSpace.get_neighbors
when multiple agents occupy the same position.Changes:
get_neighbors
explaining howinclude_center=False
affects agents at the same positioninclude_center=True
and filtering out selfContext:
When multiple agents occupy the same position and
include_center=False
is used, all agents at that position are excluded from the results - not just the center agent. This is the expected behavior, but was not clearly documented. Rather than making a breaking change to the API, this PR:include_center=True
and filtering unwanted agentsExample usage:
Closes #2592.