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

Add seed control to all examples #2496

Merged
merged 2 commits into from
Nov 13, 2024
Merged

Add seed control to all examples #2496

merged 2 commits into from
Nov 13, 2024

Conversation

quaquel
Copy link
Member

@quaquel quaquel commented Nov 11, 2024

This makes sure all example models allow the user to control the seed from the UI.

Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔴 +5.5% [+4.1%, +6.7%] 🔵 -0.2% [-0.4%, -0.0%]
BoltzmannWealth large 🔵 +0.5% [+0.1%, +1.0%] 🔵 +2.9% [+1.2%, +4.7%]
Schelling small 🔵 +0.5% [+0.3%, +0.6%] 🔵 +0.6% [+0.5%, +0.8%]
Schelling large 🔵 +0.4% [+0.0%, +0.7%] 🔵 +0.7% [-0.9%, +2.0%]
WolfSheep small 🔵 +1.0% [+0.7%, +1.3%] 🔵 -0.2% [-0.4%, +0.0%]
WolfSheep large 🔵 +0.3% [-0.6%, +1.1%] 🔵 -0.4% [-2.0%, +1.3%]
BoidFlockers small 🔵 +1.2% [+0.7%, +1.6%] 🔵 +2.6% [+1.5%, +3.7%]
BoidFlockers large 🔵 +2.5% [+2.0%, +3.0%] 🔵 +2.7% [+1.8%, +3.6%]

@EwoutH
Copy link
Member

EwoutH commented Nov 11, 2024

Like I suggested, I would like to explore if we can add this programmatically to SolaraViz itself. That way, user models also have it built-in without additional boilerplate.

@quaquel
Copy link
Member Author

quaquel commented Nov 11, 2024

Like I suggested, I would like to explore if we can add this programmatically to SolaraViz itself. That way, user models also have it built-in without additional boilerplate.

Don't let the perfect be the enemy of the good. I looked at it and its not easy to do in an elegant way. I prefer to have this in while we have an open issue to explore it further.

@EwoutH
Copy link
Member

EwoutH commented Nov 11, 2024

Let me take a quick look tomorrow, otherwise we’ll go this way.

@quaquel
Copy link
Member Author

quaquel commented Nov 12, 2024

Let me take a quick look tomorrow, otherwise we’ll go this way.

Feel free to take a look, but let me explain why it is not easy. First, some models might not need seed control. So now we need the add an option to the API to control whether the seed is included or not. Second, some users might want to use something different from the text box we use here. I just did this because we used that before and have not thought about it extensively. If we want to offer control over this to the user, we have to then offer this flexiblity in the API. One option would be to check what is in model_params passed to SoloraViz. If seed is not there, we add it in. Otherwise, we use what is there. But how does this interact with option 1? And, what if seed is set to a constant in model_params? Should we overwrite this? How do we document all these different code paths and make sure that the behavior of SolaraViz is intuitive to a user?

We can avoid all this mess by just explicitly asking users to add seed to model_params if they want to control it from the GUI.

If, at some future point, we add support for e.g., Param, we might revisit all these questions and add meaningful defaults for Parameter.

@Corvince
Copy link
Contributor

In practice I don't think it is that complicated.

  1. Check if "seed" is in model_params. If it is, just use that with the existing logic (static value => no control, UserParam => UserParam control). Done
  2. If "seed" is not in model_params, we need to check if the Model accepts "seed" at all. We already have the basics for that in the _check_params function. If it accepts "seed", add the default control. Done

The larger question I have regarding this and also this PR is how to "deactivate" the seed. With this PR (or default controls) I would expect "reset" to always yield the same starting conditions (since we fixed the seed). But for exploring models often times you want to see how it plays out with different starting conditions. Then you want "reset" to mean: Give me a new (random) model. Right now we would need to manually change the seed number. So it would good to also be able to set it to None and probably also have this as default - if you want to replicate the same starting conditions you enable it.

@quaquel
Copy link
Member Author

quaquel commented Nov 12, 2024

The larger question I have regarding this and also this PR is how to "deactivate" the seed.

I broadly agree with this idea. Ideally, we would have some kind of SeedWidget, which defaults to None. This means that on reset, a new model instance will be created with seed=None. However, if you change the value in the SeedWidget to say 42, that is used instead when creating a new model instance. As long as the SeedWidget is not set back to None, do_reset should always use the then specified value for seed. I have no idea if there is a default input component in Solara that has this behavior, but it might be worth checking.

@Corvince
Copy link
Contributor

I don't think there is, but an easy solution would be to combine a checkbox with an InputFloat. If the checkbbox ("random") is checked, the value is set to None, otherwise to the value of the InputFloat.

@quaquel
Copy link
Member Author

quaquel commented Nov 13, 2024

I suggest we approve this and open an issue about creating a dedicated widget for seed control unless someone has time to pick that up shortly.

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.

I would be hypocritical if I would delay this without a better solution ready, so here you go ;).

@Corvince
Copy link
Contributor

Maybe I'll give it a shot tonight, but this shouldn't hinder this PR anyway

@quaquel quaquel merged commit 66d2fee into projectmesa:main Nov 13, 2024
11 checks passed
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.

3 participants