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

gis: fix user warning when testing geo schelling example #189

Merged

Conversation

wang-boyu
Copy link
Member

As part of #172.

@wang-boyu wang-boyu added the gis label Sep 3, 2024
@wang-boyu
Copy link
Member Author

pre-commit.ci autofix

@EwoutH
Copy link
Member

EwoutH commented Sep 3, 2024

Thanks! Could you explain briefly what the warning was and this fixes it?

@EwoutH EwoutH mentioned this pull request Sep 3, 2024
31 tasks
@wang-boyu
Copy link
Member Author

Sure! Sorry that I didn't provide more info.

Basically the raw data contains a set of polygons, and we want to create a graph out of it, where a node represents a polygon, and a link between two polygons means that they are touch neighbors (or in other words, adjacent polygons).

But the polygons in our data can be divided into several groups, and there's no link in-between these groups. I simply keep the largest group for the model.

In server.py we hardcoded the view and zoom parameters to be [52, 12] and 4 to zoom into these regions and ignored other regions. Now that we removed other regions, these parameters aren't needed any longer.

Alternatively, we could set silence_warning to True (links: here and here) in Mesa-Geo: https://github.com/projectmesa/mesa-geo/blob/5fb2dde5e63bbfea10775af988848dbfde4ce8a2/mesa_geo/geospace.py#L297. This will silence all warnings for everyone, but I think keeping it may be useful. Hence I fixed it only for the test case of this example.

@EwoutH
Copy link
Member

EwoutH commented Sep 3, 2024

Awesome! Could you rebase/update this PR? If correct, this is the one that will turn it green!

If you find the current warnings settings too strict for Mesa-geo, we could also remove the -Werror or allow some more warnings to pass.

@wang-boyu wang-boyu force-pushed the gis/geo-schelling-user-warning branch from 232ec44 to 5ae3a58 Compare September 3, 2024 13:29
@wang-boyu
Copy link
Member Author

pre-commit.ci autofix

@wang-boyu
Copy link
Member Author

Rebased!

We can probably keep the current warning settings for now and decide later: )

@wang-boyu
Copy link
Member Author

Need to rush to a meeting and will check this later

@EwoutH
Copy link
Member

EwoutH commented Sep 3, 2024

Green!!!

Awesome.

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

update
@wang-boyu wang-boyu force-pushed the gis/geo-schelling-user-warning branch from 5ae3a58 to a304bfd Compare September 3, 2024 14:05
@wang-boyu
Copy link
Member Author

Fixed pre-commit error and squashed all commits into one.

@EwoutH
Copy link
Member

EwoutH commented Sep 3, 2024

Go ahead, I approved already

@wang-boyu wang-boyu merged commit aca30a2 into projectmesa:main Sep 3, 2024
2 of 3 checks passed
@wang-boyu
Copy link
Member Author

Thanks @EwoutH

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

Successfully merging this pull request may close these issues.

2 participants