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

Fix deterministic behavior in move_agent_to_one_of with selection="closest" #2118

Merged
merged 5 commits into from
Sep 6, 2024

Conversation

OrenBochman
Copy link
Contributor

@OrenBochman OrenBochman commented Apr 23, 2024

This PR addresses a bug in move_agent_to_one_of where agents exhibit deterministic behavior when multiple closest positions are available and selection="closest" is used. This issue caused agents to follow periodic motion patterns instead of random selection within sugar plateaus, particularly noticeable in low-vision agents.

Changes:

  • Modified move_agent_to_one_of to ensure randomness when multiple equally distant positions are found by shuffling the positions and randomly selecting from the closest ones.
  • Added a new test, test_move_agent_closest_selection_multiple, to verify that agents exhibit random behavior in scenarios with multiple closest positions.

Fixes: Agents now select randomly from multiple closest positions, preventing deterministic motion.

Closes: #2117

Copy link
Member

@EwoutH EwoutH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for catching this! You are indeed right, the closest choice is deterministic while it shouldn't be.

I have one suggestion to implement it a bit cleaner.

mesa/space.py Show resolved Hide resolved
@EwoutH EwoutH added the bug Release notes label label Apr 23, 2024
mesa/space.py Show resolved Hide resolved
Copy link
Contributor Author

@OrenBochman OrenBochman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I see that the test passes and my example that uses this code is not showing the issue anymore. SO +1

@EwoutH
Copy link
Member

EwoutH commented Apr 23, 2024

Awesome! Let’s now try to make it faster :)

@tpike3
Copy link
Member

tpike3 commented May 5, 2024

@EwoutH Do we want to wait for speed on this one or merge it and do a 2.3.1 to fix the bug?

@EwoutH EwoutH added trigger-benchmarks Special label that triggers the benchmarking CI and removed trigger-benchmarks Special label that triggers the benchmarking CI labels May 22, 2024
@EwoutH EwoutH added trigger-benchmarks Special label that triggers the benchmarking CI and removed trigger-benchmarks Special label that triggers the benchmarking CI labels Aug 17, 2024
@EwoutH EwoutH added trigger-benchmarks Special label that triggers the benchmarking CI and removed trigger-benchmarks Special label that triggers the benchmarking CI labels Sep 6, 2024
Copy link

github-actions bot commented Sep 6, 2024

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 +1.3% [+0.2%, +2.5%] 🔵 -0.1% [-0.2%, +0.1%]
BoltzmannWealth large 🔵 -0.5% [-1.0%, -0.0%] 🔵 +1.3% [-2.3%, +5.1%]
Schelling small 🔵 +0.1% [-0.1%, +0.4%] 🔵 -0.3% [-0.5%, -0.1%]
Schelling large 🔵 +0.1% [-0.4%, +0.7%] 🔵 -1.5% [-2.9%, +0.0%]
WolfSheep small 🔵 -0.7% [-1.8%, +0.4%] 🔵 -0.5% [-0.7%, -0.2%]
WolfSheep large 🔵 +0.2% [-1.3%, +1.7%] 🔵 -0.1% [-2.9%, +2.8%]
BoidFlockers small 🔵 +0.8% [+0.3%, +1.2%] 🔵 -1.7% [-2.3%, -1.1%]
BoidFlockers large 🔵 +1.0% [+0.6%, +1.5%] 🔵 -0.7% [-1.6%, +0.2%]

Copy link
Member

@EwoutH EwoutH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry that we did this die in red tape. I'm not sure if our benchmarks use this behavior, but for now correct behavior is more important than performance.

Thanks for the fix!

@EwoutH EwoutH changed the title fix for https://github.com/projectmesa/mesa/issues/2117 Fix deterministic behavior in move_agent_to_one_of with selection="closest" Sep 6, 2024
@EwoutH EwoutH merged commit 7131bdd into projectmesa:main Sep 6, 2024
11 of 12 checks passed
EwoutH pushed a commit to EwoutH/mesa that referenced this pull request Sep 24, 2024
…"closest"` (projectmesa#2118)

This PR addresses a bug in `move_agent_to_one_of` where agents exhibit deterministic behavior when multiple closest positions are available and `selection="closest"` is used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Release notes label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

move_agent_to_one_of is deterministic when pos has multiple closest choices and selection="closest"
3 participants