-
Notifications
You must be signed in to change notification settings - Fork 879
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
Conversation
Performance benchmarks:
|
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. |
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 We can avoid all this mess by just explicitly asking users to add seed to If, at some future point, we add support for e.g., Param, we might revisit all these questions and add meaningful defaults for |
In practice I don't think it is that complicated.
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 |
I broadly agree with this idea. Ideally, we would have some kind of |
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. |
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. |
There was a problem hiding this 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 ;).
Maybe I'll give it a shot tonight, but this shouldn't hinder this PR anyway |
This makes sure all example models allow the user to control the seed from the UI.