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

examples/wolf_sheep: Don't allow dumb moves #2503

Merged
merged 1 commit into from
Dec 22, 2024

Conversation

EwoutH
Copy link
Member

@EwoutH EwoutH commented Nov 13, 2024

Agents now move completely random in WolfSheep. That makes no sense whatsoever, so this PR makes them move a little bit less dumb.

  • Wolf moves to random cell with sheep on them, if available (otherwise completely random)
  • Sheep move to a random cell without a wolf (if available), and preferably with grass.

This enables sheep to actually "search" for grass, wolfs to search for sheep and sheep to not move to a cell with wolves.

More importantly, it shows of some nice selection mechanics with the cell space.

I expect the WolfSheep model to be slower, but curious by how much.

Depends on #2502.

Edit: Dynamics are quite interesting, I had this epic run where one single sheep survived endlessly until all the wolves where finally dead.

image

@EwoutH EwoutH added the example Changes the examples or adds to them. label Nov 13, 2024
@EwoutH EwoutH requested review from quaquel and Corvince November 13, 2024 16:22
Agents now move completely random in WolfSheep. That makes no sense whatsoever, so this PR makes them move a little bit less dump.

- Wolf moves to random cell with sheep on them, if available (otherwise completely random)
- Sheep move to a random cell without a wolf (if available), and preferably with grass.

This enables sheep to actually "search" for grass, wolfs to search for sheep and sheep to not move to a cell with wolves.

More importantly, it shows of some nice selection mechanics with the cell space.
@EwoutH EwoutH force-pushed the example_wolf_sheep_move branch from dcaf243 to dfbdb1a Compare November 13, 2024 20:40
@EwoutH EwoutH added trigger-benchmarks Special label that triggers the benchmarking CI and removed trigger-benchmarks Special label that triggers the benchmarking CI labels Nov 13, 2024
Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🟢 -5.5% [-7.0%, -3.9%] 🔵 -1.3% [-1.5%, -1.1%]
BoltzmannWealth large 🔵 -1.7% [-2.4%, -0.9%] 🟢 -7.8% [-8.8%, -7.1%]
Schelling small 🔵 -1.3% [-1.5%, -1.0%] 🔵 -1.0% [-1.3%, -0.8%]
Schelling large 🔵 -0.9% [-1.6%, -0.1%] 🔵 -3.1% [-4.3%, -1.8%]
WolfSheep small 🔵 -1.5% [-1.9%, -1.1%] 🔴 +82.2% [+74.6%, +89.0%]
WolfSheep large 🔵 -1.6% [-2.2%, -0.9%] 🔴 +104.6% [+101.7%, +108.4%]
BoidFlockers small 🔵 +0.2% [-0.2%, +0.7%] 🔵 +2.2% [+1.2%, +3.4%]
BoidFlockers large 🔵 -0.1% [-0.7%, +0.4%] 🔵 +1.8% [+1.3%, +2.4%]

@EwoutH
Copy link
Member Author

EwoutH commented Nov 13, 2024

Double the runtime is quite an increase. But that's good, that means we now have a meaningful step which can be profiled and optimized.

@Corvince
Copy link
Contributor

Oof, yes that's quite an increase and invites to further investigate! And this seems to be a way more interesting model dynamic

@quaquel
Copy link
Member

quaquel commented Nov 14, 2024

I have an overarching question. This model is used in the ABM Framework comparison. We, therefore, use it in our own internal benchmarks. Clearly, with the examples back in the main repo, we should not duplicate code between benchmarks and examples. This change, however, breaks the link with the ABM Framework comparison, so do we want that, and what does this imply for our own benchmark models?

Oof, yes that's quite an increase and invites to further investigate! And this seems to be a way more interesting model dynamic

It highlights a point we both have made before, no matter how fast we would make the core of mesa (e.g., via rust or cython), often the real bottleneck is user code.

As an aside, and more for my own understanding, how does the benchmarking now work with examples back in the main repo?

@EwoutH
Copy link
Member Author

EwoutH commented Dec 6, 2024

This change, however, breaks the link with the ABM Framework comparison, so do we want that, and what does this imply for our own benchmark models?

I think breaking this link is fine. The main use case of the examples is showcasing Mesa features. I think this change showcases what's possible with the Mesa Cell Space well.

It highlights a point we both have made before, no matter how fast we would make the core of mesa (e.g., via rust or cython), often the real bottleneck is user code.

While I'm not disagreeing, I don't think this applies here: Most operation are still Mesa internals (the spaces), so we're still benchmarking Mesa, and not arbitrary Python code.

As an aside, and more for my own understanding, how does the benchmarking now work with examples back in the main repo?

Their goal is to see how fast Mesa is, and to detect performance regressions or improvements when we do feature development. Since we're still benchmarking Mesa internals, sounds like it fulfills its purpose.

@EwoutH
Copy link
Member Author

EwoutH commented Dec 22, 2024

I'm merging this, because:

  1. It's an obvious improvement in behavior
  2. It demonstrates Cell Space capability not shown earlier in other examples
  3. I tested it extensively
  4. Performance can be optimized later
    (opened Optimize wolf-sheep movement performance #2565)
  5. Keeping it open increases potential conflicts with other changes to this example model.

@EwoutH EwoutH merged commit dd77c0a into projectmesa:main Dec 22, 2024
11 of 12 checks passed
@tpike3
Copy link
Member

tpike3 commented Dec 23, 2024

@EwoutH next time can you ping one of us just to make sure we get the approve on the PR. Thank you

@EwoutH
Copy link
Member Author

EwoutH commented Dec 23, 2024

You're right, will do

@EwoutH
Copy link
Member Author

EwoutH commented Dec 23, 2024

This would be a implicit change of policy though. Previously, in Mesa-examples, we didn't require explicit reviews, while we could request them.

I'm not against requiring reviews for example updates per se, but I do think this warrants a bit of a bigger discussion.

Personally I might have the stance that only core Mesa would require explicit reviews. Other stuff is non-critical, and we can trust each others judgement to request a review when they are nessesary.

We might have to accept the occasional revert. I don't think that is a bad thing, it means we're finding our path and learning things.

quaquel pushed a commit that referenced this pull request Dec 28, 2024
Agents now move completely random in WolfSheep. That makes no sense whatsoever, so this PR makes them move a little bit less dump.

- Wolf moves to random cell with sheep on them, if available (otherwise completely random)
- Sheep move to a random cell without a wolf (if available), and preferably with grass.

This enables sheep to actually "search" for grass, wolfs to search for sheep and sheep to not move to a cell with wolves.

More importantly, it shows of some nice selection mechanics with the cell space.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
example Changes the examples or adds to them.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants