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

Refactoring mesa.visualization.components.matplotlib #2401

Closed
quaquel opened this issue Oct 23, 2024 · 4 comments · Fixed by #2430
Closed

Refactoring mesa.visualization.components.matplotlib #2401

quaquel opened this issue Oct 23, 2024 · 4 comments · Fixed by #2430
Milestone

Comments

@quaquel
Copy link
Member

quaquel commented Oct 23, 2024

I have taken a closer look at the solara matplotlib visualization support in MESA and tried to refactor it a bit because of various redundancies. However, I ran into a few issues. Basically, how the data is gathered before plotting is dependent on the type of space that is to be visualized. For example, there is no relevant position data when visualizing networks. Offsets to x and y are added for grids. Moreover, for discrete spaces, we have multiple implementations of both old-style and new-style (e.g., HexSingleGrid, HexMultiGrid, HexGrid). I believe it is quite easy to abstract away whether you are plotting an old-style or a new-style space. They mainly differ with respect to how you get the agents to plot and their (x, y) coordinates, which is easily wrapped into custom generators (these generators are probably also useful for the altair side of things, so I'll probably add a components.util module).

In an attempt to make sense of this all, I have come to the following breakdown:

OrthogonalGrid = SingleGrid | MultiGrid | OrthogonalMooreGrid | OrthogonalVonNeumannGrid
HexGrid = HexSingleGrid | HexMultiGrid | HexGrid
ContinousSpace
Network = NetworkGrid | mesa.experimental.grid_space.Network 
VoronoiGrid

draw_orthogonal_grid(space: OrthogonalGrid, agent_portrayal: Callable, propertylayer_portrayal: Callable)
    ...

draw_hex_grid(space: HexGrid, agent_portrayal: Callable, propertylayer_portrayal: Callable):
    ...

draw_network(space: Network, agent_portrayal: Callable, propertylayer_portrayal: Callable):
    ...

draw_continuous_space(space: ContinousSpace, agent_portrayal: Callable):
    ...


draw_voroinoi_grid(space: VoronoiGrid, agent_portrayal: Callable, propertylayer_portrayal: Callable):
    ...

I have several questions:

  1. Does the breakdown into types of spaces make sense?
  2. Did I miss anything?
  3. Property layers are currently implemented in DiscreteSpace for new-style spaces. So they are available in all new style spaces, including Network and VoronoiGrid. Should property grids be moved from DiscreteSpace to Grid? This would mimic old-style spaces. I also question whether property layers are presently meaningful for networks or Voronoi meshes. (I want to focus the conversation on improving the current visualization code, so please don't bring up ideas on how to expand property layers to also work in e.g., VoronoiGrid or continuous spaces).

If this brake down makes sense, I'll reorganize the code accordingly and start writing some tests for it.

@EwoutH
Copy link
Member

EwoutH commented Oct 23, 2024

Interesting, this means we're also changing the agent_portrayal and propertylayer_portrayal from dicts to Callables? How would such a Callable look in your view?

@quaquel
Copy link
Member Author

quaquel commented Oct 23, 2024

Not sure I follow. At the moment, agent_portrayal is already a callable (see boltzmann example). I haven't yet looked at propertylayer_portrayal, given the name I just implicitly assumed it also was a callable.

It might be possible to do Callable|dict, so a callable that returns a dict for each agent or a dict with defaults for all agents.

@EwoutH
Copy link
Member

EwoutH commented Oct 23, 2024

Sorry, you'r right, late evening brain fart.

The overal setup looks good.

As discussed in #2407, each space has its particularities. But some are similar, except for a very tiny detail. Do we need to go proper OOP on this and make a general Draw class, and subclass form there?

Also since, spaces and visualisation are tightly coupled we could consider only updating for the new (cell) spaces, since it might simplify things. Then we target this refactor for Mesa 3.1.

@quaquel
Copy link
Member Author

quaquel commented Oct 24, 2024

As discussed in #2407, each space has its particularities. But some are similar, except for a very tiny detail. Do we need to go proper OOP on this and make a general Draw class, and subclass form there?

I am not sure yet. The interdependencies are tricky. It might be possible to use two base classes: one for drawing the type of space and one for gathering the data (old vs. new-style). However, ContinuousSpace and old-style NetworkGrid don't have the same interface for getting agents as _Grid. One possibility, which I have done in a local branch, is to add def __iter__(self) -> Iterator[GridContent]: to ContinuousSpace and NetworkGrid. This would make all old-style spaces identical for getting the agents.

The difference between getting agents from ContinuouSpace and new-style spaces is something that warrants attention for 3.1. I am inclined to add a refactored version of ContinuousSpace to experimental and use this to address at long last #1278, #1354, and #2149. The approach taken in #2292 is the way forward for this long-standing question.

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

Successfully merging a pull request may close this issue.

2 participants