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

Draw space(#2638) #2657

Closed
wants to merge 5 commits into from
Closed

Conversation

aarav-shukla07
Copy link
Contributor

Summary

The draw_space method in Mesa's visualization module fails when there are no agents on the grid. This bug prevents the visualization from rendering correctly in models with empty grids, limiting the usability of the visualization tools for certain scenarios.

Bug / Issue

  • Issue: #2638

  • Description: The draw_space method assumes there is at least one agent to determine the grid's properties. When the grid is empty, this assumption causes the method to fail or behave incorrectly.

  • Expected Behavior: The draw_space method should render the grid correctly, even when there are no agents.

  • Actual Behavior: The method fails or produces incorrect visualizations when the grid is empty.

Implementation

Modified the draw_space function

Testing

All the tests get passed from the test_space.py of tests folder .

Additional Notes

  • Potential Side Effects: None expected. The changes only affect the visualization of empty grids and do not modify the core functionality of the grid or agents.

  • Dependencies: None.

  • Related Work: This fix is part of ongoing efforts to improve Mesa's visualization tools and ensure they work reliably in all scenarios.

Copy link

github-actions bot commented Feb 1, 2025

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🟢 -5.5% [-6.3%, -4.7%] 🔵 -0.2% [-0.4%, +0.0%]
BoltzmannWealth large 🟢 -8.1% [-9.2%, -7.0%] 🔵 +1.8% [-0.3%, +3.7%]
Schelling small 🟢 -8.7% [-8.9%, -8.4%] 🔴 +6.5% [+6.3%, +6.8%]
Schelling large 🟢 -9.3% [-9.7%, -9.0%] 🟢 -48.2% [-48.6%, -47.7%]
WolfSheep small 🟢 -16.0% [-16.5%, -15.5%] 🟢 -50.2% [-52.4%, -47.9%]
WolfSheep large 🟢 -12.7% [-14.0%, -11.2%] 🟢 -60.7% [-62.1%, -59.0%]
BoidFlockers small 🔵 -0.3% [-1.0%, +0.4%] 🟢 -26.1% [-26.7%, -25.5%]
BoidFlockers large 🔵 +1.3% [+0.4%, +2.2%] 🔴 +79.1% [+77.6%, +80.5%]

@aarav-shukla07 aarav-shukla07 changed the title Draw space Draw space(#2638) Feb 1, 2025
@aarav-shukla07 aarav-shukla07 deleted the draw_space branch February 1, 2025 13:06
@aarav-shukla07 aarav-shukla07 restored the draw_space branch February 1, 2025 13:07
@aarav-shukla07 aarav-shukla07 reopened this Feb 1, 2025
@Sahil-Chhoker
Copy link
Contributor

@aarav-shukla07 I think you should take a look at implementation and Jan's comment on #2640. Also take a look at the discussion #2642.

@@ -1,4 +1,4 @@
name: example-environment
name: mesa-tutorials
Copy link
Member

Choose a reason for hiding this comment

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

please keep PRs focussed, this is a separate issue and thus does not belong in this PR. I suggest reverting all changes to this file

@@ -108,7 +108,9 @@ def draw_space(
space,
agent_portrayal: Callable,
propertylayer_portrayal: dict | None = None,
ax: Axes | None = None,
ax_grid: Axes | None = None,
Copy link
Member

Choose a reason for hiding this comment

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

I am not convinced that this is the ideal solution for an improved API. You now have to pass the axes multiple times in the most common use case. I suggest we hatch out the API design in #2640 first before opening a PR.

if hasattr(space, "agents") and space.agents:
if ax_agents is None:
ax_agents = ax_grid
draw_agents(space, agent_portrayal, ax=ax_agents)
Copy link
Member

Choose a reason for hiding this comment

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

This is a good name for a new method. But I don't see an implementation of draw_agents at the moment?

@aarav-shukla07 aarav-shukla07 closed this by deleting the head repository Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants