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

SolaraViz Updates #2299

Merged
merged 2 commits into from
Sep 19, 2024
Merged

SolaraViz Updates #2299

merged 2 commits into from
Sep 19, 2024

Conversation

Corvince
Copy link
Contributor

Some more work on the SolaraViz Frontend. Highlights

  1. Much better docstrings
  2. Removed make_text function. It was used like make_text(renderer), but now you can use renderer directly as a component
  3. Merged Play/Pause button and disabled step button while model is running
  4. Reorganised the Sidebar to look like this
    image

Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 -0.8% [-1.6%, +0.0%] 🔵 -0.4% [-0.6%, -0.2%]
BoltzmannWealth large 🔵 +0.1% [-0.8%, +1.2%] 🔵 +1.8% [-0.7%, +5.7%]
Schelling small 🔵 +0.1% [-0.2%, +0.4%] 🔵 -0.7% [-0.9%, -0.5%]
Schelling large 🔵 +0.8% [+0.2%, +1.4%] 🔵 -1.2% [-2.6%, +0.4%]
WolfSheep small 🔵 +0.1% [-0.6%, +0.7%] 🔵 -0.1% [-0.4%, +0.1%]
WolfSheep large 🔵 +1.6% [+0.9%, +2.3%] 🔵 +0.9% [-2.4%, +4.3%]
BoidFlockers small 🔵 -1.3% [-1.6%, -0.8%] 🔵 -1.0% [-1.5%, -0.5%]
BoidFlockers large 🔵 -1.2% [-1.5%, -1.0%] 🔵 -0.5% [-0.9%, -0.0%]

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.

Looks great!

Not directly for this PR, but are you still supporting a name change (we were considering ModelExplorer right)?

@rht
Copy link
Contributor

rht commented Sep 18, 2024

Removed make_text function. It was used like make_text(renderer), but now you can use renderer directly as a component

Which objects are allowed/not allowed? make_text was specific to string objects. Would float work?

@rht
Copy link
Contributor

rht commented Sep 18, 2024

I didn't voice this concern in the past: but before this PR, the reset button was dangerously close to the play button, that people could accidentally fat-fingered it. But now it seems safer.

@rht
Copy link
Contributor

rht commented Sep 18, 2024

One of the tests in test_solara_viz.py failed.

@Corvince
Copy link
Contributor Author

Which objects are allowed/not allowed? make_text was specific to string objects. Would float work?

Good question, I don't know. I would guess only strings work, but I will test this out

@Corvince Corvince merged commit b9088dd into main Sep 19, 2024
9 of 10 checks passed
@Corvince Corvince deleted the solara-viz-updates branch September 19, 2024 13:35
@rht
Copy link
Contributor

rht commented Sep 19, 2024

In this case, it's fine since I haven't provided any change requests, so the agreement is implicit, but in the future, I'd appreciate if the visualization PR gets merged when I explicitly approve, since @EwoutH hadn't dev-ed in the viz area yet.

@Corvince
Copy link
Contributor Author

Sounds fine to me! But yes, since you already replied without any change requests I thought you are fine with the changes. And appreciated the heads-up to the failing test

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

Successfully merging this pull request may close these issues.

3 participants