Skip to content

Validate HexGrid torus dimensions#2951

Merged
quaquel merged 5 commits intomesa:mainfrom
ShreyasN707:grid-wiring
Dec 18, 2025
Merged

Validate HexGrid torus dimensions#2951
quaquel merged 5 commits intomesa:mainfrom
ShreyasN707:grid-wiring

Conversation

@ShreyasN707
Copy link
Copy Markdown
Contributor

Summary

This PR prevents undefined HexGrid configurations by disallowing toroidal grids with odd dimensions, which previously resulted in asymmetric neighbor wiring.

Bug / Issue

Fixes #2811.

HexGrid with torus=True and an odd width or height leads to incorrect neighbor wrapping, where connections are not bidirectional. This configuration was previously allowed without validation, causing undefined behavior.

Implementation

-Added validation in HexGrid._validate_parameters to require even width and height when torus=True.

-A ValueError is raised for invalid configurations to prevent undefined topology.

Testing

-Added tests covering:

- Invalid toroidal grids with odd dimensions (error raised)

- Valid toroidal grids with even dimensions

- Non-toroidal grids with odd dimensions

- Ran the full test suite to ensure no regressions.

Additional Notes

This introduces validation for a previously unsupported configuration, causing affected models to fail fast with a clear error instead of producing inconsistent behavior.

@github-actions
Copy link
Copy Markdown

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 +2.3% [+1.5%, +3.2%] 🔵 +0.1% [-0.1%, +0.2%]
BoltzmannWealth large 🔵 +0.7% [-0.0%, +1.5%] 🔵 +4.4% [+1.8%, +7.6%]
Schelling small 🔵 -0.7% [-1.6%, +0.4%] 🔵 +0.1% [-0.4%, +0.7%]
Schelling large 🔵 -3.8% [-7.1%, -1.8%] 🔵 -0.3% [-3.1%, +3.2%]
WolfSheep small 🔵 -4.6% [-6.8%, -2.6%] 🟢 -3.3% [-3.6%, -3.1%]
WolfSheep large 🔵 +2.7% [-0.4%, +8.1%] 🔵 -0.3% [-1.7%, +1.7%]
BoidFlockers small 🔵 -0.7% [-1.1%, -0.4%] 🔵 -1.6% [-1.7%, -1.4%]
BoidFlockers large 🔵 -0.8% [-1.3%, -0.3%] 🔵 -1.2% [-1.4%, -1.0%]

@ShreyasN707
Copy link
Copy Markdown
Contributor Author

@EwoutH @quaquel Please review my PR.

@github-actions
Copy link
Copy Markdown

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 +2.4% [+1.6%, +3.1%] 🔵 +0.6% [+0.5%, +0.8%]
BoltzmannWealth large 🔵 +2.4% [-0.0%, +6.1%] 🔵 +1.3% [-1.3%, +4.2%]
Schelling small 🔵 +0.4% [-0.6%, +1.3%] 🔵 +0.7% [+0.1%, +1.3%]
Schelling large 🔵 +0.3% [-2.9%, +3.7%] 🔵 -0.8% [-1.8%, +0.3%]
WolfSheep small 🔵 -4.0% [-5.9%, -2.3%] 🔵 -0.8% [-1.1%, -0.6%]
WolfSheep large 🔵 -0.6% [-1.7%, +0.6%] 🔵 -1.6% [-3.2%, +0.0%]
BoidFlockers small 🔵 +0.7% [+0.3%, +1.1%] 🔵 +1.4% [+1.1%, +1.7%]
BoidFlockers large 🔵 +0.1% [-0.7%, +0.7%] 🔵 +0.6% [+0.3%, +0.9%]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you also add this additional behavior and value error to the docstring?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure!

Copy link
Copy Markdown
Member

@quaquel quaquel left a comment

Choose a reason for hiding this comment

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

One minor addition, but looks fine otherwise. Thanks.

@ShreyasN707 ShreyasN707 requested a review from quaquel December 17, 2025 12:08
@quaquel quaquel merged commit c773979 into mesa:main Dec 18, 2025
13 checks passed
@quaquel
Copy link
Copy Markdown
Member

quaquel commented Dec 18, 2025

Merged and thanks

@EwoutH
Copy link
Copy Markdown
Member

EwoutH commented Dec 18, 2025

Thanks for the PR, and the review!

(please do check each merged PR has an appropriate release notes label)

@EwoutH EwoutH added the enhancement Release notes label label Dec 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Release notes label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cells keep cache after removing connections (3.2.0 version)

3 participants