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

Optimize model execution to stop immediately when required #2699

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

HMNS19
Copy link
Contributor

@HMNS19 HMNS19 commented Feb 22, 2025

This PR updates the do_step function to exit early when running.value is False. Previously, it would continue executing n steps even after running was set to stop. Now, it stops immediately when running.value becomes False.

This applies to models running in both ModelController and SimulatorController.

Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 +0.6% [+0.1%, +1.1%] 🔵 -0.1% [-0.3%, +0.2%]
BoltzmannWealth large 🔵 +0.2% [-0.4%, +0.8%] 🔵 -1.6% [-2.4%, -0.8%]
Schelling small 🔵 -0.4% [-0.6%, -0.2%] 🔵 -0.5% [-0.7%, -0.4%]
Schelling large 🔵 -0.6% [-0.8%, -0.3%] 🔵 -0.5% [-1.9%, +0.7%]
WolfSheep small 🔵 -1.2% [-1.4%, -1.0%] 🔵 -1.0% [-1.1%, -0.8%]
WolfSheep large 🔵 -1.0% [-1.5%, -0.5%] 🔵 -0.8% [-1.1%, -0.5%]
BoidFlockers small 🔵 +1.2% [+0.3%, +2.1%] 🔵 +3.2% [+3.0%, +3.4%]
BoidFlockers large 🔵 +0.9% [+0.5%, +1.2%] 🔵 +2.4% [+2.1%, +2.6%]

@HMNS19
Copy link
Contributor Author

HMNS19 commented Feb 22, 2025

I noticed that the checks didn’t pass in my PR due to an AttributeError in the test. The issue seems to be that model.running isn’t set in the mock object for simulator models. Since I added a feature to stop the model when running is False, the test fails because it tries to access model.running, which doesn’t exist in the mock.

Would it make sense to update the test setup to include model.running = True, or should we handle this differently?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant