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

viz: stop running and disable buttons when model.running is False #2332

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

wang-boyu
Copy link
Member

This PR stops calling model.step() from the frontend when model.running is False. Play and step buttons are also disabled.

@wang-boyu wang-boyu added the enhancement Release notes label label Sep 28, 2024
Copy link

Performance benchmarks:

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

@Corvince
Copy link
Contributor

Looks good to me!

@EwoutH
Copy link
Member

EwoutH commented Sep 29, 2024

How/when should/can I use this?

@wang-boyu
Copy link
Member Author

Oh sorry I should've provided more context.

The current solara viz keeps calling model.step() and updating plots, even when the model stops itself, in models such as the Schelling example:

def step(self):
    # model logic
    ...
    if self.happy == len(self.agents):
        self.running = False

This PR stops the frontend when model.running becomes False, and disables the play and step buttons.

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.

Awesome, sounds like an useful feature!

@EwoutH
Copy link
Member

EwoutH commented Sep 30, 2024

@quaquel want do you think about the whole self.running concept?

@quaquel
Copy link
Member

quaquel commented Sep 30, 2024

@quaquel want do you think about the whole self.running concept?

A currently underdeveloped feature that needs to be implemented better. It is used with self.run_model, which is also strange and creates a typing mess. I. have looked at it a few times without coming up with a clean solution. My rough idea is to have some stopping_condition callable, which can be parameterized by arguments to self.run_model, and when the stopping condition becomes true, self.running is automatically set to false. By default this stopping_condition might simply be tied to steps and be parameterized with n for the number of steps. If this stopping_condition is a method on Model, you would have a meaningful default while making it user-extendable,

@rht
Copy link
Contributor

rht commented Sep 30, 2024

Minor question:

I forgot about the old behavior of SolaraViz. Which one is it:
1.has it been this way but changed in #2299, then this PR fixes it
2. the bug has always been there even before #2299

This only affects whether it should be communicated as a bug fix or a new feature in the release note.

@wang-boyu
Copy link
Member Author

It's probably included in #2263? I didn't try it but there were some usages of model.value.running. It was replaced with playing.value in #2282.

@wang-boyu wang-boyu merged commit 4e45300 into projectmesa:main Oct 1, 2024
11 of 14 checks passed
@wang-boyu
Copy link
Member Author

Merged! Pls feel free to change the label if anyone thinks there's a better one.

@wang-boyu wang-boyu deleted the viz/stop-when-model-stops branch October 1, 2024 17:57
@rht
Copy link
Contributor

rht commented Oct 2, 2024

It's a bug fix. The old JupyterViz does check the model.running:

def do_play():
model.running = True
while model.running:
do_step()
.

@rht rht added bug Release notes label and removed enhancement Release notes label labels Oct 2, 2024
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.

5 participants