Added multipage functionality #2827
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Performance benchmarks:
|
WalkthroughAdds a page parameter to plotting component factories, forwards it through backend dispatch, and changes backends to return (component, page). Updates Solara visualization to a multi-page, tabbed layout: components are provided as (component, page) tuples, normalized to page 0 for legacy inputs, and rendered per-page grids. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SolaraViz
participant ComponentsView
participant Grid per Page
User->>SolaraViz: components (list) or "default"
SolaraViz->>SolaraViz: Normalize to (component, page) tuples (default page=0)
SolaraViz->>ComponentsView: components[(component, page)...]
ComponentsView->>ComponentsView: Group by page, order pages
ComponentsView->>Grid per Page: Build per-page grid layouts
User->>ComponentsView: Switch tab (page N)
ComponentsView->>Grid per Page: Render grid for page N
sequenceDiagram
participant Caller
participant make_plot_component
participant Matplotlib
participant Altair
Caller->>make_plot_component: measure, post_process, backend, page, kwargs
alt matplotlib
make_plot_component->>Matplotlib: make_mpl_plot_component(..., page, **kwargs)
Matplotlib-->>make_plot_component: (MakePlotMatplotlib, page)
else altair
make_plot_component->>Altair: make_altair_plot_component(..., page, **kwargs)
Altair-->>make_plot_component: (MakePlotAltair, page)
end
make_plot_component-->>Caller: (component_callable, page)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
mesa/visualization/components/altair_components.py (1)
454-475: make_altair_plot_component now returns a tuple, breaking make_plot_component’s API
The Altair variant of the plotting factory was changed toreturn (MakePlotAltair, page), whereas the Matplotlib version still returns just a factory function. As a result,
- Calls to
make_plot_component(..., backend="altair")now receive a(func, page)tuple instead of a single callable- Existing user code that invokes the returned value as a function will break at runtime
Locations requiring your attention:
- mesa/visualization/components/altair_components.py, lines 454–475
• Change the return value ofmake_altair_plot_componentso it returns only the factory function (e.g. embedpagein the closure)
— or—- mesa/visualization/components/init.py, in
make_plot_componentaltair branch
• Unpack(MakePlotAltair, page)and return just the function (or document/handle the tuple consistently for both backends)Please align the Altair API with Matplotlib’s to avoid this breaking change.
mesa/visualization/components/matplotlib_components.py (1)
107-130: Breaking API change:make_mpl_plot_componentnow returns(MakePlotMatplotlib, page)This update aligns the Matplotlib helper with Altair’s behavior but will break any existing code that expects a lone function.
Please address the following:
- In
mesa/visualization/components/matplotlib_components.py(lines 107–130), update the docstring to state that the function returns a(component_factory, page)tuple.- In
mesa/visualization/components/__init__.py, audit all calls tomake_mpl_plot_componentand ensure callers unpack both elements of the tuple.- Update your changelog/release notes and bump the major version to signal this breaking change.
🧹 Nitpick comments (1)
mesa/visualization/solara_viz.py (1)
61-63: Complex type annotation may cause compatibility issuesThe type annotation for
componentsis quite complex and might cause issues with older Python versions or type checkers. Consider simplifying or usingtyping.Unionfor better compatibility.- components: list[tuple[reacton.core.Component], int] - | list[tuple[Callable[[Model], reacton.core.Component], 0]] + components: list[tuple[reacton.core.Component, int]] + | list[tuple[Callable[[Model], reacton.core.Component], int]]
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
mesa/visualization/components/__init__.py(2 hunks)mesa/visualization/components/altair_components.py(2 hunks)mesa/visualization/components/matplotlib_components.py(2 hunks)mesa/visualization/solara_viz.py(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
mesa/visualization/components/__init__.py (2)
mesa/visualization/components/matplotlib_components.py (1)
make_mpl_plot_component(107-130)mesa/visualization/components/altair_components.py (1)
make_altair_plot_component(454-475)
mesa/visualization/solara_viz.py (4)
mesa/visualization/components/altair_components.py (2)
make_altair_space(27-61)agent_portrayal(50-51)tests/test_solara_viz.py (1)
agent_portrayal(129-130)tests/test_solara_viz_updated.py (1)
agent_portrayal(126-127)mesa/visualization/space_renderer.py (2)
post_process(383-389)post_process(392-399)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-benchmarks
🔇 Additional comments (12)
mesa/visualization/components/altair_components.py (1)
457-457: LGTM! Page parameter properly added and documentedThe new
pageparameter is correctly added with a sensible default value of 0, and the docstring has been updated to document it.Also applies to: 465-465
mesa/visualization/components/__init__.py (2)
77-77: LGTM! Page parameter properly added with clear documentationThe
pageparameter is correctly added with a default value of 0, and the docstring clearly explains its purpose.Also applies to: 86-86
93-99: Backend plot components updated as expectedBoth
make_mpl_plot_componentandmake_altair_plot_componentnow include thepageparameter in their signatures and return tuples ((MakePlotMatplotlib, page)and(MakePlotAltair, page), respectively), so no further changes are needed.mesa/visualization/components/matplotlib_components.py (1)
110-110: LGTM! Page parameter properly documentedThe
pageparameter is correctly added with a default value of 0 and properly documented in the docstring.Also applies to: 118-118
mesa/visualization/solara_viz.py (8)
27-27: LGTM! Required import for defaultdictThe
collectionsimport is correctly added to support the newdefaultdictusage inComponentsView.
84-87: Documentation updated correctly for multipage supportThe docstring has been properly updated to explain the new tuple format with page numbers.
123-131: Default component correctly wrapped with page 0The default Altair space component is properly wrapped as a tuple with page 0, maintaining consistency with the new multipage structure.
151-151: Space component correctly inserted with page 0The space component from the renderer is properly inserted at the beginning with page 0.
375-382: Good backward compatibility handlingThe code properly handles backward compatibility by converting non-tuple components to
(component, 0)tuples.
384-395: Efficient page organization with gap fillingThe code efficiently groups components by page and fills in missing page indices to maintain sequential tab order. This ensures a clean UI even when pages are specified non-contiguously.
403-419: Well-implemented layout synchronizationThe layout synchronization logic properly handles adding new pages and removing deleted ones, maintaining state consistency.
422-443: Clean implementation of tabbed multipage layoutThe implementation using Solara's Tabs and TabItems is clean and properly handles the per-page grid layouts with drag-and-drop functionality preserved.
|
I like the look of this. Great stuff. 2 quick thoughts.
|
Thanks!
This is just my observation, but when we look only at the plot component, the steps increase very quickly—so quickly that the space component can’t keep up. When it switches to the space one, the step counters don’t match, so two values exist at the same time. Since that can’t happen, it crashes.
My goal was to keep the indexing consistent, and it felt weird to put page 10 exactly after page 1 (if they were passed), therefore I just decided to make empty pages, if that's not what the user wants they can adjust the index themselves. |
My thought on this, is that the empty pages are ok, because the parameter is stating "put this chart on this page". Correct me if I am wrong @Sahil-Chhoker (having not tested it yet) if we expand your example, then the below would put the space on page 0, plot1 on page 1 and then the two custom component on page 2, correct? If my understanding is correct, one possible way to make it more explicit would be to make the kwarg -- One question, do users have the ability to change the page title - so instead of "Page 0" it could say "Sugarscape", page 1 could say "Price Index" etc... |
Yes!
Change
Not yet, but if you want that is possible if I just add another field to the tuple and rework the page naming logic. |
That would be great if users can define their page titles, please do. As for the |
|
I've tried adding page name but this introduces too many reactive variables in SolaraViz, making it unstable. I believe we don't want that, what do you say @tpike3? |
No stable is definitely better |
|
@Sahil-Chhoker I am not sure there is anything you can do about this but there definitely was some challenges switching between plots, while it was running. If there is no ability to refine, I would add a warning to pause the model before switching views. Could you please also add use of pages to an example (I think sugarscape would be the obvious, with the warning) as well as add it to a tutorial (with the warning about switching pages unless paused, unless it can be refined). Only other question, as I didn't experiment with it, is I had to use the Form here then we can merge and I will start the Mesa 3.3. release process. |
I'll see what I can do.
Sure.
I would have done the same but in case of custom components we are passing a tuple of component and page, and tuple don't support passing keyword arguments inside them. {
"component": CustomCom,
"page": 3,
}Let me know if I should change the tuples with dict. |
There’s no way to optimize page switching while the model is running. If we add a warning to pause the model before switching, I suggest we also implement lazy loading, as it significantly improves performance. |
There was a problem hiding this comment.
Excellent work as always.
@quaquel , @EwoutH , @jackiekazil
Any comments or issues, before I merge. This will wrap up @Sahil-Chhoker GSOC project and cause release of Mesa 3.3
Summary
We can have different components on different pages in the solara dashboard now.
How it looks:
Motive
Instead of cluttering all kinds of plots on a single interface, we can now divide them across multiple pages, making the dashboard cleaner and more presentable.
Implementation
ComponentsViewsolara component was changed to process (component, page) and display them according to the page.If no page is passed
page=0is default.Negative values for pages can also exist (like
page=-1) and if the two plots are let's saypage=1andpage=10while being nothing in between,9empty pages will be generated to keep the indexing constant.SpaceRendereris always on thepage=0.Usage Examples
We can pass
pagevariable tomake_plot_componentto change the page it renders on.If making an custom component and want to render on page other than
0, have to pass the component like(component, 1)where component is the custom component and1being the page number.Additional Notes
I tried adding in lazy loading (loading only the page that is selected) but when page is changed from a computationally lighter page to a heavier one solara crashes.
Summary by CodeRabbit