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

Add support for drawing discrete grids #2386

Merged
merged 8 commits into from
Oct 21, 2024

Conversation

quaquel
Copy link
Member

@quaquel quaquel commented Oct 18, 2024

Add support for drawing experimental Grid Spaces (i.e. HexGrid, OrthogonalMooreGrid, OrthogonalVonNeumannGrid).

Closes #2377. This code needs further refinement (e.g., duplication of agent potrayal code from voroinoi grid, offsets for hexgrids, and adding experiment NetworkGrids).

@quaquel quaquel requested a review from EwoutH October 18, 2024 16:03
@quaquel quaquel requested a review from Corvince October 18, 2024 16:03
Copy link

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 -0.8% [-2.2%, +0.7%] 🔵 +0.5% [+0.3%, +0.7%]
BoltzmannWealth large 🔵 -1.4% [-1.8%, -1.0%] 🔵 -2.5% [-3.0%, -1.9%]
Schelling small 🔵 -1.6% [-1.9%, -1.4%] 🔵 -1.4% [-1.6%, -1.1%]
Schelling large 🔵 -0.9% [-1.9%, -0.0%] 🟢 -5.2% [-6.3%, -3.9%]
WolfSheep small 🔵 -1.0% [-1.3%, -0.8%] 🔵 -0.9% [-1.1%, -0.7%]
WolfSheep large 🔵 +0.8% [-0.0%, +1.8%] 🔵 +0.3% [-1.7%, +2.7%]
BoidFlockers small 🔵 -1.0% [-1.7%, -0.2%] 🔵 -0.9% [-1.8%, +0.1%]
BoidFlockers large 🔵 -1.1% [-1.8%, -0.4%] 🔵 -1.2% [-1.8%, -0.7%]

@EwoutH
Copy link
Member

EwoutH commented Oct 19, 2024

Uninstall your current mesa, pip install . -e and then solara run app.py is what I found out working.

@quaquel
Copy link
Member Author

quaquel commented Oct 19, 2024

Uninstall your current mesa, pip install . -e and then solara run app.py is what I found out working.

Thanks! This gave me the idea for a slightly different approach: insert the correct version of mesa into sys.path at the top. Based on this, I can confirm that this code works as intended.

Let me know what else would be needed to complete this PR since its my first contribution to the visualization side of things.

@EwoutH
Copy link
Member

EwoutH commented Oct 20, 2024

insert the correct version of mesa into sys.path at the top. Based on this, I can confirm that this code works as intended.

That's the other obvious way to go, both have their advantages based on what your working on.

@Corvince I'm also quite new to this stack, could you please review this one?

Copy link
Contributor

@Corvince Corvince left a comment

Choose a reason for hiding this comment

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

I think this PR is very fine and a good first step. I pre-approve this, just please also add OrthogonalVonNeumann

@@ -11,7 +11,7 @@
from matplotlib.figure import Figure

import mesa
from mesa.experimental.cell_space import VoronoiGrid
from mesa.experimental.cell_space import OrthogonalMooreGrid, VoronoiGrid
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also work for OrthogonalVonNeumannGrid

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, I'll fix that! Just need to figure out how to capture this in the match case statement.

I guess it even will work for hexgrids (although we might want to visualize these a bit differently with offsets for every odd row.

@quaquel quaquel merged commit 42043d3 into projectmesa:main Oct 21, 2024
11 of 12 checks passed
@@ -52,16 +52,20 @@ def SpaceMatplotlib(
if space is None:
space = getattr(model, "space", None)

if isinstance(space, mesa.space._Grid):
_draw_grid(space, space_ax, agent_portrayal, propertylayer_portrayal, model)
Copy link
Member

Choose a reason for hiding this comment

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

Hi @quaquel, I understand that this is already merged. May I ask whether it is intentional to remove the call to _draw_grid() here? Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess I made a mistake. I'll put in a PR to fix this asap.

Thanks for the post-merge review!

Copy link
Member Author

Choose a reason for hiding this comment

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

and fixed via #2398

Copy link
Member

Choose a reason for hiding this comment

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

Great! Thanks!

@EwoutH
Copy link
Member

EwoutH commented Oct 23, 2024

One improvement for this function would be to use the _get_agent_data when drawing the new grids. I got a weird error, and using _get_agent_data would have prevented and handled that. I wrote it up here:

@quaquel
Copy link
Member Author

quaquel commented Oct 23, 2024

One improvement for this function would be to use the _get_agent_data when drawing the new grids. I got a weird error, and using _get_agent_data would have prevented and handled that

I looked at that and I could not use it because that method is hard coded to only work with _Grid: for agents, pos in space.coord_iter().

@EwoutH EwoutH added feature Release notes label experimental Release notes label labels Oct 26, 2024
@quaquel quaquel deleted the visualize_experiment_grid branch November 4, 2024 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Release notes label feature Release notes label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visualize Cell Space
4 participants