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

fixing the draw_orthogonal_grid function #2668

Closed
wants to merge 1 commit into from

Conversation

sanika-n
Copy link
Contributor

@sanika-n sanika-n commented Feb 7, 2025

Bug

I noticed that using make_mpl_space_component to create a plot, always created weird patterns in the background when the model used orthogonal grids. The red dots are agents, but the gray patterns were actually the rendition of the orthogonal grid by the draw_orthogonal_grid function.

Before

image

The logic seems to be correct, but the grid is being drawn using ":", this makes it easy to confuse between the horizontal and the vertical lines causing visually confusing renditions. To resolve this, I just changed ":" to "-" in the code.

After

image

To replicate an issue of this kind you could change draw_grid to True in sugarspace

Copy link

github-actions bot commented Feb 7, 2025

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 -2.6% [-4.1%, -1.3%] 🔵 -0.6% [-0.8%, -0.4%]
BoltzmannWealth large 🔵 -2.1% [-4.6%, -0.4%] 🔵 -2.5% [-3.4%, -1.5%]
Schelling small 🔵 +0.5% [+0.2%, +0.8%] 🔵 -0.3% [-0.4%, -0.2%]
Schelling large 🔵 +0.9% [+0.4%, +1.3%] 🔵 +2.0% [+0.7%, +3.0%]
WolfSheep small 🔵 -0.4% [-0.9%, +0.2%] 🔵 +0.0% [-0.3%, +0.4%]
WolfSheep large 🔵 -1.1% [-2.4%, -0.0%] 🔵 -2.6% [-4.3%, -0.9%]
BoidFlockers small 🔵 -2.5% [-2.9%, -2.0%] 🟢 -4.3% [-4.5%, -4.2%]
BoidFlockers large 🔵 -2.3% [-2.8%, -1.9%] 🟢 -3.8% [-4.0%, -3.7%]

@quaquel
Copy link
Member

quaquel commented Feb 8, 2025

This is related to #2438 and, more broadly, to #2640 and #2642. So rather than shift from one predetermined style to another, I favor changing the API so that the user can explicitly control this (but with meaningful defaults).

@sanika-n
Copy link
Contributor Author

So do you recommend that I add the feature to mention the type of grid, or should I just close this pull request?

@quaquel
Copy link
Member

quaquel commented Feb 10, 2025

I suggest describing this issue in #2642 and then closing this PR as the best way forward.

@sanika-n sanika-n closed this Feb 10, 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.

3 participants