api reorganization

[quaquel]

This reorganizes the component code by separating solara components (in altair_components and matplotlib_components) and the more broadly useful space drawing functions (mpl_space_drawing).

I have a few questions/thoughts at this point:

1. We renamed the Matplotlib components to make_space_component and make_plot_component. Neither name indicates clearly that these are Matplotlib specific. There are at least 3 solutions:

   1. Rename them again and at mpl or so to their name
   2. Have generic make_space_component and make_plot_components and add a backend kwarg to control whether Matplotlib or Altair is used. I like this from an API point of view. It gives the user a single entry point when creating components, abstracting the backend (i.e., the plotting library) away. However it also suggests that both backends will always behave in the same way, and I am not sure that we can guarantee that and even whether that is desirable.
   3. Don't worry about it for now, and revisit this once altair_components is being refactored.

2. I kept the mpl_space_drawing module in mesa.visualization.components. It might make more sense to promote this to mesa.visualization since they are also useful outside of solara components.

3. I added the mpl_space_drawing module to the visualization docs but subsumed it under Matplotlib-based components. Depending on how we handle the previous questions, the docs might need to be reorganized a bit.

Closes #2434.

[github-actions]

Performance benchmarks:

Model	Size	Init time [95% CI]	Run time [95% CI]
BoltzmannWealth	small	🔵 -0.9% [-1.7%, -0.0%]	🔵 -0.1% [-0.2%, +0.0%]
BoltzmannWealth	large	🔵 -0.3% [-0.7%, +0.1%]	🔵 -0.7% [-1.5%, -0.0%]
Schelling	small	🔵 -0.4% [-0.8%, +0.0%]	🔵 -1.1% [-1.3%, -0.9%]
Schelling	large	🔵 +0.7% [-0.4%, +1.7%]	🔵 -0.5% [-2.3%, +1.7%]
WolfSheep	small	🔵 +0.1% [-0.3%, +0.9%]	🔵 +0.8% [+0.6%, +1.0%]
WolfSheep	large	🔵 -0.6% [-1.3%, +0.2%]	🔵 -1.5% [-2.5%, -0.4%]
BoidFlockers	small	🔵 +0.4% [-0.3%, +1.1%]	🔵 -0.9% [-1.6%, -0.4%]
BoidFlockers	large	🔵 +0.4% [-0.5%, +1.0%]	🔵 +0.1% [-0.5%, +0.7%]

[Corvince]

On the first question I would go for 2), generic API with specific backends. Also I have been thinking about possibly removing Altair completely for 3.0 and then add the backend keyword and the Altair drawing functions in 3.1.

This would give us more time to have a proper Altair implementation for all spaces and probably also adding interactive elements to the Altair plots, e.g. clicking on agents to get additional information/ change the model. This isn't hard to do in Altair, but I would like to have some time to have this truly stable.

[quaquel]

On the first question I would go for 2), generic API with specific backends. Also I have been thinking about possibly removing Altair completely for 3.0 and then add the backend keyword and the Altair drawing functions in 3.1.

We were broadly in agreement to declare visualization.components experimental. That would give us the same possibility for implementing the Altair side properly for 3.1. One concern I have at the moment is that I am not convinced it is possible to abstract away the backend with the current status of the API. Declaring the entire components namespace experimental would give us more leeway if needed.

[Corvince]

We were broadly in agreement to declare visualization.components experimental

Was there already an agreement on that? Then that is fine as well. I proposed that somewhere, but the alternative was to have a good component API, which you single handedly provided in the last days :D
So personally I would be happy with declaring it stable already. But declaring it experimental would also work

[quaquel]

Was there already an agreement on that? Then that is fine as well.

I think @EwoutH agreed with your suggestion. I want to keep it experimental, with the understanding that we aim to implement Altair with a compatible API. We will only break the API it if we can't find a clean solution for altair within the confines of the API established for matplotlib.

I'll do more reorganization of the code, such as moving make_space_component and make_plot_components up, while adding backend specific versions to altair_compoments and `matplotlib_components. This will lay the groundwork for 3.1.

[quaquel]

Ok, I have reorganized things further.

1. In matplotlib_components and altair_components there are now make_(backend)_(type)_component functions. Note that Altair only has space at the moment
2. I added generic make_space_component and make_plot_component that take a backend keyword argument (next to all the other kwargs). This gives a common backend-independent API.
3. I updated all examples to use the generic make_space_component and make_plot_component, although they all use the matplotlib backend. As part of the Altair work, we should move a few of them to Altair.
4. I updated tests and docs were needed to cover everything.

Note that some of the stuff is implemented with an eye towards the altair rework. If @EwoutH agrees, it would make sense to add the experimental warnings as well (I can do it here or in a follow up PR).

I think this PR is ready for review.

[EwoutH]

I can dive into this Monday, if you would like to move forward before then I trust both your judgement.

[EwoutH]

What's the minimal input you currently need to unblock this PR?

[EwoutH]

A few simple points:

1. I find it a bit inconsistent that matplotlib sometimes gets shortened to mpl and sometimes doesn't.
2. components.matplotlib_components feels redundant. What if we only allow importing directly from components.matplotlib, and not add them to a lower level __all__?
3. I don't see that much added value in make_space_component with an keyword argument now. I don't see us supporting more than a few backends, so I would suggest explicit make_space_matplotlib and make_space_altair functions. We can document and type hint those better, and check in each of that type of space is supported.

[quaquel]

1. I find it a bit inconsistent that matplotlib sometimes gets shortened to mpl and sometimes doesn't.

I found that the names became rather long if I did not do it, but I am fine with changing it.

2. components.matplotlib_components feels redundant. What if we only allow importing directly from components.matplotlib, and not add them to a lower level all?

Not sure I follow. At a minimum, the name matplotlib should not be used because it can give rise to namespace clashes when also importing matplotlib itself, the same applies to altair. It is what motivated this PR in the first place (#2434).

3. I don't see that much added value in make_space_component with an keyword argument now. I don't see us supporting more than a few backends, so I would suggest explicit make_space_matplotlib and make_space_altair functions. We can document and type hint those better, and check in each of that type of space is supported.

I followed @Corvince's suggestion based on my question at the start of this PR. I am fine with dropping it and only keeping the backend-specific ones. The benefit, however, of doing it this way is that it enforces us to have a common API for both backends. This is something that is quite desirable from a user point of view.

I haven't bothered with comprehensive type hinting yet. I started doing it using @overload, but I need to do a bit more reading on how that interacts with different values for keyword arguments.

[EwoutH]

Sounds it was well thought out, if you and Vincent align, I'm good

[quaquel]

Ok, let's see what @Corvince makes of this PR.

[Corvince]

I agree with this PR very much. On question 2) I would agree to promote mpl_space_drawing to mesa.visualization. Its neither component nor solara viz specific.

On matplotlib vs mpl: I agree that matplotlib names become a bit unwieldy due to their length. But I also think mpl is something to stumple over. Still I think mpl is the bit better solution, since I don't expect it being used that much directly. API-wise I think a simple user-facing solution is to just use from mesa.visualization import draw_space, make_space_component. Its not really necessary to import the invidiual make_mpl_space_component, so its not very user-facing.

Following this logic it might also make sense to define draw_space outside of mpl_space_drawing, since from a user-perspecive its more important to describe what we want to do (draw space) than to describe how we want to do it (using matplotlib)

[Corvince]

Unrelated, and I wanted to create a separate PR but its hard to get the timing right with respect to the reorganizing of the files:

draw_space currently takes the post_process argument. Since this works on the ax but we already return the ax I don't think thats necessary. I think it should be just an option for make_space_component. If you agree maybe you can just change it to this PR? (and then apply post process in make_mpl_space_component on the return value of draw_space)

[quaquel]

Ok, I have moved mpl_space_drawing.py from mesa.visualization.components into mesa.visualization. I have updated the API docs to reflect this move. I also slightly changed what is importable from mesa.visualization. The current version is backward compatible with main, while including the new generic make_space_component, make_plot_component, and draw_space.

I think that only leaves open a choice on draw_space

[quaquel]

draw_space currently takes the post_process argument. Since this works on the ax but we already return the ax I don't think thats necessary. I think it should be just an option for make_space_component. If you agree maybe you can just change it to this PR? (and then apply post process in make_mpl_space_component on the return value of draw_space)

good point, I have made that change here.

[wang-boyu]

Thanks for the work here. It is pretty similar to the refactoring part in #2453, with the main differences in API styles (OO vs. functional).

Some comments from my side -

On the matplotlib plotting functions:

1. We both extracted matplotlib plotting functions into a separate file, here mpl_space_drawing.py. I would suggest to extract measure plottting function there too, apart from draw_space, which might be useful for other Viz that needs to plot measures using matplotlib. In addition, if mpl_space_drawing.py contains these measure plotting function, then a more generic name such as mpl_drawing.py might be better, since it's not just about space drawing.
2. Also regarding the matplotlib plotting functions, I would suggest to limit the amount of public APIs, such as draw_orthogonal_grid, draw_hex_grid, which are prone to future updates and hence breaking changes.

On the SolaraViz components:

1. Similarly to implement VideoViz to record model runs in a video #2453, all SolaraViz stuff are kept in solara_viz.py and the components folder. Main different is where we put make_space_component. Here it is in components/__init__.py, which makes sense, since it's about creating a Solara component.
   I put it in solara_viz.py because there's also a make_space_component for VideoViz. As a result, we can have

   from mesa.visualization.video_viz import VideoViz, make_space_component, make_measure_component
   from mesa.visualization.solara_viz import SolaraViz, make_space_component, make_measure_component

I think we overall agree on most of the stuff. Personally I don't have strong preferences on either OO-style or functional style. In summary, we could perhaps view the SolaraViz feature as some sort of "frontend" while matplotlib and altair are its "backend". So we could have in the future (apologies for the poor drawing)

graph LR
    A(SolaraViz) --- B(matplotlib)
    A(SolaraViz) --- C(altair)
    A(SolaraViz) --- D(other_backend)
    E(VideoViz) --- B(matplotlib)
    F(StreamlitViz) --- B(matplotlib)

Loading

The "frontend"s share common API in their creations:

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")],
)

but differ in how there are used:

page   # simply use the page to render Solara webpage

viz.record(steps=50, filepath="schelling.mp4")

[quaquel]

Thanks for the feedback @wang-boyu. Some quick reactions

I would suggest to extract measure plottting function there too.

I checked the code but at the moment there is no detailed measure plotting helper functions in matplotlib_components.py. If they are added in the future, I agree, that whatever is useful beyond solara should be in a seperate matplotlib/altair plotting file.

Also regarding the matplotlib plotting functions, I would suggest to limit the amount of public APIs,

I agree. That's why I removed them from the __init__ in mesa.visualization. So they are available, but you have to dig into the api docs to find them. We could add a leading underscore to them if desired, but at the moment I find that a bit of overkill. Also, it seems that @Corvince, @EwoutH, and me all broadly agree that mesa.visualization is work in progress and experimental (but with minimum API breaking changes unless absolutely necessary).

I think we overall agree on most of the stuff. Personally I don't have strong preferences on either OO-style or functional style. In summary, we could perhaps view the SolaraViz feature as some sort of "frontend" while matplotlib and altair are its "backend"

I want to hash out an OO version but not for MESA 3.0. I also completely agree on the altair/matplotlib/plotly(?) backend idea.

[wang-boyu]

I checked the code but at the moment there is no detailed measure plotting helper functions in matplotlib_components.py.

@quaquel Sorry I meant this function. In #2453 it got changed to

    fig, ax = plt.subplots()
    renderer = MeasureRendererMatplotlib(
        measure=measure, post_process=post_process, save_format=save_format
    )
    renderer.draw(model, ax)

where MeasureRendererMatplotlib is extracted into matplotlib drawing file (equivalent to the mpl_space_drawing.py file here). I thought that something similar could be done here too.

[quaquel]

Ok that clarifies it. Indeed, the body of PlotMatplotlib could be separated into its own helper function. However, at the moment, I don't see a lot of value in doing it. Its just a simple if else block for handling the different kinds of possible arguments. I suggest we leave it here for now. Once this is merged, and it would be helpful for VideoViz, we can allways move it over.

A broader issue is that, at the moment, out of the box plotting is limited to line plots over time. I want to explore and see if there are default plots that we might want to support directly. Moreover, at the moment, the line plotting is tightly coupled to the datacollector. I want to find a way to separate these completely. In short, ample work to still be done after this PR.

[Corvince]

Fully agree here with @quaquel . Currently there isn't much added benefit, but we should eventually provide not just line graphs and be independent of the data collector

[wang-boyu]

I suppose we're waiting for @Corvince's comments now? Also curious about what @EwoutH thinks regarding our changes.

[quaquel]

I suppose we're waiting for @Corvince's comments now? Also curious about what @EwoutH thinks regarding our changes.

Yes, I would like to get @Corvince's perspective on the draw_space function and then this is ready to be merged. Given that @wang-boyu, @Corvince and I agree on this refactoring, I think @EwoutH will happily accept our view, but I'll chat with him today about it.

[Corvince]

Would that not create redundancy with the make_space_component because then we would end up with make_space_component and draw_space both of which would take a backend kwarg. Conceptually, I agree that they are different, and draw_space might be used more widely (e.g., in VideoViz #2453).

I don't think it would be redundant. I expect draw_space to be called more often, in videoviz or just interactively in a notebook, while building up a model (and seeing the current state). make_space_component is much more specific.
But for now this is mostly about code organization. We can also decide on that later, move draw_space somewhere else, add a backend argument and still it wouldn't break the API if imported from mesa.visualization.

[Corvince]

Was there anything else to comment on from my side before we move on?

[quaquel]

Was there anything else to comment on from my side before we move on?

No this is it. Thanks!

But for now this is mostly about code organization. We can also decide on that later, move draw_space somewhere else, add a backend argument and still it wouldn't break the API if imported from mesa.visualization.

These are good points. I'll think about it a bit more this morning and merge it later today.

[EwoutH]

From the diff, it looks good, and I have no objections.

Really appreciate the work on this from all of you.

[EwoutH]

Do we need to update the migration guide after this PR?

[quaquel]

good point, the guide is still valid but will raise some deprecation warnings so better to update it.

[Corvince]

(But note that doc updates can still be submitted after the release, which work-wise should be prioritized)

[quaquel]

docs default to the stable branch, so ideally we get it in (or have to to patch releases just for docs).

[EwoutH]

Is the visualization.__init__.py fully updated as it needs to be? make_space_altair is deprecated but imported, and make_mpl_space_component and make_altair_space are not imported, is that intended?

[quaquel]

make_plot_component and make_space_component should be sufficient because they take a backend kwarg that allows you to control the backend. This reduces the public API.