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

Bugs in SolaraViz? #2452

Closed
quaquel opened this issue Nov 2, 2024 · 2 comments
Closed

Bugs in SolaraViz? #2452

quaquel opened this issue Nov 2, 2024 · 2 comments
Labels
bug Release notes label front end

Comments

@quaquel
Copy link
Member

quaquel commented Nov 2, 2024

I started working on #2427 and have run into three issues:

  1. If the model takes one or more arguments and those arguments are not part of the user-controllable parameters that are set from the UI, stuff will crash immediately
  2. Strangely, it crashes immediately even though we started SolaraViz with a valid model instance, so this points to a second bug: at the moment, the model shown after starting the UI is not the same as the original model with which the UI was initialized. Again, this is easy to reproduce. In the boltzman wealth model, in app.py, start the model with say a width and height of 20. Next, start Solara, and in the GUI, you'll see that the actual model has reverted back to 10 by 10 (as specified in model_params).
  3. Seed is currently handled differently from other parameters. With model: Move random seed and random to __init__ #1940 merged, this is no longer needed, and seed can just be integrated into the user-specifiable parameters.
@quaquel quaquel added bug Release notes label front end labels Nov 2, 2024
@Corvince
Copy link
Contributor

Corvince commented Nov 3, 2024

Yes from looking at the code I can confirm these bugs. If user-setable parameters are provided the ModelCreator is set up and initializes a new model. This should not happen initially. But this raises the question how to work with models that need parameters, but those are not provided. Should reset just be disabled or should we create a helpful error message?

@quaquel
Copy link
Member Author

quaquel commented Nov 3, 2024

  1. We should add to the docs that it is recommended to only use keyword argument in Model.__ini__
  2. We should have an informative error. It might even be possible to use reflection to check the signature of the __init__ and raise the exception immediately. I would advocate against aligning inputs from the UI with the order of arguments in the __init__: when in doubt refuse the temptation to guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Release notes label front end
Projects
None yet
Development

No branches or pull requests

2 participants