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

Viz: Refactor Matplotlib plotting #2430

Merged
merged 77 commits into from
Oct 30, 2024
Merged

Conversation

quaquel
Copy link
Member

@quaquel quaquel commented Oct 27, 2024

This refactors the existing matplotlib plotting code in line with #2401. Note that this refactor is useful regardless of how #2389 is resolved. This makes what is there better, cleaner, and complete for all spaces without changing the user-facing API. The return of agent_potrayal is now the same for all spaces because they all plot agents with matplotlib's scatter.

In short, it is built around dedicated plotting functions for different types of spaces while abstracting away the difference between old-style and new-style discrete spaces. These plotting functions return an axes instance, making it easier to test and use each plotting function separately from Solara.

Any field returned by agent_portrayal that is not used results in an a user warning.

I also changed the behavior of orthogonal grids a bit. The current code adds an offset to the x and y coordinates, while the grid lines are drawn on integer values. Instead, I change the view limits and draw the grid lines on 0.5, 1.5, etc. I believe this better reflects the desired behavior (i.e., the agent on the coordinate (1,1) is shown centered (1, 1) while the grid lines nicely go around it.

Hexgrids were not supported before. This is now added although property layers are not yet shown correctly. All other stuff works.

Network plotting is refactored substantially. Users can specify which layout algorithm to use. The network edges are optional and drawn analogously to the grid in other discrete spaces.

Tests for the individual plotting functions have been added. These only check if the code runs, not if the plots are correct. This has been checked visually.

Copy link

Performance benchmarks:

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

@quaquel quaquel added feature Release notes label enhancement Release notes label visualisation labels Oct 27, 2024
@quaquel quaquel linked an issue Oct 27, 2024 that may be closed by this pull request
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.

Lot's of interesting stuff in here!

This makes what is there better, cleaner, and complete for all spaces without changing the user-facing API.

Nice!

A debatable choice made here is that agent_portrayal must return keys that are valid for ax.scatter. That is 'c', 's' and 'marker'. I am not sure about this.

I would for now support the "old" values with a simple translation dict {"color": "c, "Layer", "zorder", ...}. Ideally we shouldn't have to modify our examples.

I also changed the behavior of orthogonal grids a bit. The current code adds an offset to the x and y coordinates, while the grid lines are drawn on integer values. Instead, I change the view limits and draw the grid lines on 0.5, 1.5, etc. I believe this better reflects the desired behavior (i.e., the agent on the coordinate (1,1) is shown centered (1, 1) while the grid lines nicely go around it.

This is a difficult one. It will help maintaining smaller differences between continuous and discrete spaces (since 1, 1 is now identical to 1.0, 1.0), but it might be a bit inconvenient with grids, cell agents, PropertyLayers, etc. Have you checked those now correctly align?

Hexgrids were not supported before. There is a draft implementation

Really awesome!

mesa/visualization/components/matplotlib.py Outdated Show resolved Hide resolved
mesa/visualization/components/matplotlib.py Outdated Show resolved Hide resolved
@quaquel
Copy link
Member Author

quaquel commented Oct 27, 2024

Thanks for the feedback!

would for now support the "old" values with a simple translation dict {"color": "c, "Layer", "zorder", ...}. Ideally we shouldn't have to modify our examples.

So "Layer" and "Zorder" are already not supported in the current code, and so also not supported here. This is in fact one of the reasons why I am included to raise a value error for any element in the agent portrayal dict that is not supported. Errors should not be passed over in silence.

Not sure about the dict. With 3.0, we can break backward compatability. I prefer a clear choice over maintaining 2 different labels for each valid element in the dict. I personally like the clear one-to-one relation between arguments for ax.scatter and what can go into the dict.

This is a difficult one. It will help maintaining smaller differences between continuous and discrete spaces (since 1, 1 is now identical to 1.0, 1.0), but it might be a bit inconvenient with grids, cell agents, PropertyLayers, etc. Have you checked those now correctly align?

I have tested this but not for property layers. I want to take a closer look at those anyway also because of hexgrids.

@quaquel
Copy link
Member Author

quaquel commented Oct 27, 2024

I added some first tests. These only make sure that the plotting code (so the draw_x functions) runs, not that the code is correct. I have used a notebook to check the resulting figures. Most of it looks good.

Some quick updates:

  1. I added width and height properties to voroinoi grid. This makes it possible to use the same scatter code as continuous and orthogonal spaces. I noticed various other issues with vorionoi grid that need their own PR. For example, there is a cell_coloring_property that has no business being there. Moreover, these grids are similar to continuous spaces in that they should have an xmin, xmax, ymin, ymax, next to width and height. I'll try to open a separate PR that only cleans up VoronoiGrid.
  2. The current network plotting code is a mess. Check the agent_portrayal function in the virus_on_a_network example. I will clean this up so that agent_portrayal behaves the same across all spaces. I already alluded to this idea in Giving the user more control in solara visualization of spaces #2389.
  3. Hexgrid plotting runs, but I still need to confirm it's actually correct.
  4. The current mesa.space.NetworkGrid is just horrible. The API is completely different from the other old-style spaces and very slow. I can't wait to deprecate this stuff in favor of mesa.experimenta.cell_space.Network.

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.

This PR is remarkably clean for it's size and scope, excellent work. Few minor comments, but they are really nitpicks.

I especially like how the arguments in the dict are now handled and how everything is documented. Thanks!

mesa/examples/basic/conways_game_of_life/app.py Outdated Show resolved Hide resolved
Comment on lines +38 to +40
OrthogonalGrid = SingleGrid | MultiGrid | OrthogonalMooreGrid | OrthogonalVonNeumannGrid
HexGrid = HexSingleGrid | HexMultiGrid | mesa.experimental.cell_space.HexGrid
Network = NetworkGrid | mesa.experimental.cell_space.Network
Copy link
Member

Choose a reason for hiding this comment

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

This seems something we want to maintain throughout Mesa (like a space hierarchy). Can we put this on a more central place?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just typing information, not sure how that could be centralized.


def draw_property_layers(ax, space, propertylayer_portrayal, model):

def draw_property_layers(
Copy link
Member

Choose a reason for hiding this comment

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

Note to self: Make singular draw_property_layer function, which returns a single axes. Multiple axes with one (semi-transparent) propertylayer each, and optionally a space, can than be combined into a single figure.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I understand the idea

@quaquel quaquel merged commit 92054d7 into projectmesa:main Oct 30, 2024
10 of 12 checks passed
@quaquel quaquel deleted the matplotlib_refactor branch October 30, 2024 08:30
@EwoutH EwoutH changed the title Matplotlib refactor Viz: Refactor Matplotlib plotting Oct 30, 2024
@EwoutH EwoutH added this to the v3.0 milestone Oct 30, 2024
EwoutH pushed a commit that referenced this pull request Oct 30, 2024
With #2430 merged, the visual of wolf sheep changed a bit. This updates the png on the landing page to be consistent again with the example.
@EwoutH EwoutH added breaking Release notes label and removed enhancement Release notes label labels Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Release notes label feature Release notes label visualisation
Projects
None yet
4 participants