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.

Sign up for GitHub

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: Don't continue playing when a model is reset #1796

Merged
merged 1 commit into from
Sep 16, 2023
Merged

fix: Don't continue playing when a model is reset #1796

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion mesa/experimental/jupyter_viz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,25 @@ def ModelController(
):
playing = solara.use_reactive(False)
thread = solara.use_reactive(None)
# We track the previous step to detect if user resets the model via
# clicking the reset button or changing the parameters. If previous_step >
# current_step, it means a model reset happens while the simulation is
# still playing.
previous_step = solara.use_reactive(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you seen the use previous hook? Might be usable here

https://solara.dev/api/use_previous

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tried, but with that, the simulation will always step once after the reset. Not sure why. At least this PR works, and the code is not that much different from if it were using use_previous.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's still a bit verbose. The step+1 got me curious and I was playing around a bit.

I think a simpler solution would be to use use_previous on the model and then look if the previous model is equal to the current model. If it's not set playing to False and reset current step.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's problematic to store 2 copies of model with different specs at any given time. Consumes 2x RAM.


def on_value_play(change):
if model.running:
Copy link
Contributor

Choose a reason for hiding this comment

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

We should check for model.running and playing.value. That way you won't see any unwanted step+1 on reset

if previous_step.value > current_step and current_step == 0:
# We add extra checks for current_step == 0, just to be sure.
# We automatically stop the playing if a model is reset.
playing.value = False
elif model.running:
do_step()
else:
playing.value = False

def do_step():
model.step()
previous_step.value = current_step
set_current_step(model.schedule.steps)

def do_play():
Expand Down