Skip to content

Bugfix ModelCreator for required model parameters and user adjusted model parameters #2780

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

Merged
merged 2 commits into from
May 17, 2025

Conversation

Holzhauer
Copy link
Contributor

Summary

For models with required parameters, providing a user input in model_params (passed to SolarViz()) still raises a ValueError: Missing required model parameter. Passing user_params instead of fixed_params to _check_model_params resolves the issue.

Bug / Issue

Because model params are only checked for fixed_params, user adjusted parameters are not considered and mistakenly considered as missing.

Implementation

  • pass user_params instead of fixed_params to _check_model_params
  • rename user_params as return from split_model_params to user_adjust_params

Testing

see tests/test_solara_viz.py::test_model_creator()

Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 +2.1% [+1.0%, +3.3%] 🔵 +0.3% [+0.1%, +0.5%]
BoltzmannWealth large 🔵 -0.8% [-2.3%, +0.3%] 🔵 +0.9% [-0.7%, +2.8%]
Schelling small 🔵 -0.5% [-0.6%, -0.3%] 🔵 -0.6% [-0.8%, -0.3%]
Schelling large 🔵 +1.9% [-0.9%, +6.7%] 🔵 -2.7% [-4.0%, -1.3%]
WolfSheep small 🔵 -0.1% [-0.4%, +0.2%] 🔵 -1.3% [-1.5%, -1.1%]
WolfSheep large 🔵 -3.1% [-5.0%, -1.4%] 🟢 -10.7% [-13.3%, -8.4%]
BoidFlockers small 🟢 -4.3% [-5.1%, -3.5%] 🔵 -0.4% [-0.7%, +0.0%]
BoidFlockers large 🔵 -2.2% [-2.9%, -1.5%] 🔵 -0.2% [-0.6%, +0.3%]

Copy link
Member

@tpike3 tpike3 left a comment

Choose a reason for hiding this comment

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

This is great @Holzhauer, this has needed improving for awhile!

@tpike3 tpike3 merged commit 2e6ab85 into projectmesa:main May 17, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants