-
Notifications
You must be signed in to change notification settings - Fork 917
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
implement VideoViz to record model runs in a video #2453
base: main
Are you sure you want to change the base?
Conversation
I will look at the code tomorrow, but the video looks lit! 🔥 |
This comment was marked as off-topic.
This comment was marked as off-topic.
I'll take a detailed look tomorrow. I like the video! This does, however, interact with #2447, and I am, at face value, not yet convinced by the OO style over the function style API for now (which also interacts with some of the ideas in #2389). @wang-boyu, how do you view the interdependencies between this pr and #2447 and #2389? |
Oh sorry I didn't mean to replace #2447. I wanted to have VideoViz for a project I'm working on and ended up with this PR. To me the OO-style vs. functional style is really just a design choice -
So if we end up with funcitonal-style API instead, VideoViz can be built based on that as well. I think my main point here is to isolate matplotlib rendering out of SolaraViz so they it can be re-used by VideoViz. Regarding #2389, I really like the ideas of having the more powerful APIs, instead of defining dictionaries in a |
No worries. Having VideoViz seems like a good idea, would close #885, and it's just very useful. I only want to make sure that we maintain some coherence in the development. So, in my understanding, this pr adds:
This directly overlaps with #2447. It seems we agree that separating the
I saw your comment in #2389 and hope to return to that asap. For MESA 3.0, I would counsel against this refactoring because it is breaking the established API. In my view, we should hash out an OO API for space and plot drawing, have this as experimental in 3.1, and see where it goes. I suggest we have the conversation on this in #2389. A major challenge, I expect, will be to abstract away the backend, given how radically different Altair and Matplotlib are.
This directly overlaps with #2447, although I moved these functions into the
Yes, I agree, and this was also on my to-do list for after #2447.
The core of this PR, in my understanding, and, yes, a very good idea. So, I propose the following next steps.
|
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.
great to see the examples used for this as well.
One concern, not for this pr, is how to add these additional files to the docs.
mesa/visualization/video_viz.py
Outdated
) | ||
|
||
|
||
def make_measure_component( |
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.
In the other parts of the visualization, we shifted to just calling it a plot component; what are the reasons for using the, in my view, ambiguous label of measure here?
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.
Thanks for asking. I kept the impression from the old (now experimental) SolaraViz:
mesa/mesa/experimental/solara_viz.py
Line 58 in 69571b4
if "Space" in layout_type: |
mesa/mesa/experimental/solara_viz.py
Line 72 in 69571b4
elif "Measure" in layout_type: |
where there's a card titled "Space" and another titled "Measure". "Plot" seems a bit amgiguous for me because we can have a plot for space, a plot for measure, and possible plots for other stuff. I might have missed some discussions when we made the change.
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 just followed a suggestion from @Corvince in #2443.
I agree on the ambiguity, but I contend that space is different in kind from, say, line plots, histograms, or other visualizations of data created by a model. So, for now, I am fine with separeting space from other kinds of plots.
Since we are still hashing out and refining the visualization style, I am fine with this distinction at the moment.
figsize: Optional figure size in inches (width, height) | ||
grid: Optional (rows, cols) for custom layout. Auto-calculated if None. | ||
""" | ||
# Check if FFmpeg is available |
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.
This is perhaps for users the most tricky part. I agree on the error handling here. I would suggest also adding this to the module level docstring.
mesa/visualization/video_viz.py
Outdated
@@ -0,0 +1,184 @@ | |||
"""Video recording components for Mesa model visualization.""" |
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 suggest making clear that this only works with the matplotlib backend
I will give you two a bit of time to align on this topic, let me know if a fresh perspective is appreciated! |
I thought I already commented on this PR, but it seems I only wanted to do so. Nice work with the output, the video looks very clearn. On the topic of OO vs FP: There are use-cased for both approaches, but I don't think OO is very beneficial in this case. The only available method is "record", so why not use it directly? #Instead of
viz = VideoViz(
model,
components,
title="Schelling's Segregation Model",
)
viz.record(steps=50, filepath="schelling.mp4")
# Just do
record(
model,
components,
title="Schelling's Segregation Model",
steps=50,
filepath="schelling,mp4
) This leaves more room for users to compose how they want. To replicate your interface def my_record(steps, filepath):
return record(
model,
components,
title="Schelling's Segregation Model",
steps,
filepath
)
my_record(50, "schelling.mp4") Or maybe you only want to change the model def my_record(model):
return record(
model=model,
components=components,
title=f"${model.name} Segregation Model",
steps=50,
filepath=f"${model.name}.mp4"
) |
Now that #2447 is merged, I'll update this PR accordingly. |
Thanks @Corvince for your feedback. I mainly wanted to have similar APIs in creating the different Viz "frontends": page = SolaraViz(
model,
[make_space_component(agent_portrayal=agent_portrayal),
make_measure_component("happy")],
)
viz = VideoViz(
model,
[make_space_component(agent_portrayal=agent_portrayal),
make_measure_component("happy")],
) where the usages might differ: page # simply use the page to render Solara webpage
viz.record(steps=50, filepath="schelling.mp4") |
Rebased to main branch and updated for #2447:
|
I looked through the code and have at the moment a few high-level reactions
I don't mind this change, but I am not fully convinced it is needed.
Given 1. this makes perfect sense.
Here I have some questions. On the Solara side, we call things components because that is what they are in Solara. You borrow that terminology here, even though this code, as far as I can tell, will only work with matplotlib. So, does this component naming make sense, given that In my understanding of the API based on the typing information, a component is a callable that is called with the model and the ax as arguments. But do you need the complexity of videoviz specific I understand your motivation for going OO to keep the similarity with SolaraViz. I don't mind doing it that way, but again, I wonder whether just accepting that it will be matplotlib specific might not open up a more generic and powerful API. For example, you hard-code the grid layout at the moment. What if the user wants to change this? For example, in a 16:9 aspect ratio slide, I might like to have a 3x2 grid, while on a web page, I might prefer 2x3. What about having a gridspec optional keyword argument on the To be clear, I needed to make a short mp4 of a mesa model today, and having this in would have saved me quite some time. I am just not yet fully convinced about some of the design choices made here. |
Thanks for the comments!
If we want the same plot for measures in SolaraViz and VideoViz, without extracting a common method
I'm hoping there could be more backend options for VideoViz in the future, other than matplotlib. It'll be easier to develop and integrate new backend into both SolaraViz and VideoViz with the current API design, hopefully.
Yes, these
Yeah it'll be good to allow users to configure grid layout, something I also wondered here: #2236 (reply in thread). I would prefer some common ways of addressing this, and a common API between SolaraViz and VideoViz would benefit from it, e.g.: # `make_` functions are imported from mesa.visualization
layout = SomeLayoutCustomization(
make_space_component(agent_portrayal=agent_portrayal),
make_measure_component("happy")
)
page = SolaraViz(model, layout)
# here `make_` functions are imported from mesa.visualization.video_viz
layout = SomeLayoutCustomization(
make_space_component(agent_portrayal=agent_portrayal),
make_measure_component("happy")
)
viz = VideoViz(model, layout) so that when users replace I'm also wondering whether it'll be possible to make the Or maybe something even further: model_explorer = ModelExplorer(
model,
SomeLayoutCustomization(
make_space_component(agent_portrayal=agent_portrayal),
make_measure_component("happy")
),
)
# renders Solara app if used in Jupyter notebook or called in `solara run app.py`
model_explorer
# exports a video
model_explorer.record(steps=10, filepath="my_amazing_model.mp4") I guess this is beyond the scope of this PR? Apologies for being off topic. But this is also close to what @Corvince proposed above, to have just a single |
Thanks for the extensive reaction.
This is the core of the discussion. Do we want to make it trivial to swap between the UI and recording a movie? I see great value in this for ease of use and API simplicity. So, ideally, all you would have to change is
Do we have any description of creating mp4's with altair or plotly (a possible other backend supported in solara)? If we have, then I am less sceptical about the potential of having If we don't have any clear direction for supporting both Altair and matplotlib in VideoViz, it might actually be confusing to a user to have the same API for both Playing devil's advocate, another separate reason for not bothering with the same API for VideoViz |
One motivation for me is that I will need to run the same model with many different parameter settings, and I don't want to do manual recordings for all of them. Back on the APIs, I think the benefit of current design is that it is open for future backends, without making breaking changes. What are the proposed changes to the APIs? I'm a bit lost in these discussions : ) Is it to change to a single |
Fair point and good motivation
Ok, but I still would like to understand the plausibility of this being realized in the future. It is great to design for future extensions, but if those future extensions are not plausible or the API becomes convoluted, why bother?
I haven't discussed or commented on the actual API in much detail yet. For me, the second point here is the key. If there is a plausible path to mp4 with Altair, I would comment on this API differently compared to a situation where the focus is on matplotlib only. (and sorry for being perhaps a bit annoying, I really like the functionality provided here, but since it is new functionality, I believe it is worth it to have a slightly longer back and forth on how to best provide this functionality). |
Summary
This PR consists of three commits:
Various
draw_space()
functions are moved and refactored into a separatematplotlib_renderer.py
file.Previously:
Now:
and for measures:
This is mainly to extract matplotlib plotting functions/classes out of Solara components, so that they can be re-used by other visualization methods, such as the VideoViz in the third commit. Besides, I don't think it changes how SolaraViz is used.
make_space_component
andmake_plot_component
are moved fromvisualization.components.matplotlib
intovisualization.solara_viz
, with abackend
option to choose frommatplotlib
oraltair
. Default space drawing is changed fromaltair
tomatplotlib
.Implement a
VideoViz
that has similar APIs toSolaraViz
with an example for the basic Schelling model. Essentially, runningcd mesa/examples/basic/schelling python3 video.py
will produce a
video.mp4
file:schelling.mp4
As mentioned in 1), VideoViz uses common matplotlib renderers extracted from SolaraViz.
Usage Examples
In file
mesa/examples/basic/schelling/video.py
.