-
-
Notifications
You must be signed in to change notification settings - Fork 186
Improve Caching and Replay Example #102
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
Conversation
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
examples/caching_and_replay/model.py
Outdated
|
|
||
| self.schedule = mesa.time.RandomActivation(self) | ||
| self.grid = mesa.space.SingleGrid(width, height, torus=True) | ||
| self.grid = mesa.space.SingleGrid(height, width, torus=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be width first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to follow the benchmark schelling model, and in that height is first.
But on checking mesa.space.SingleGrid width is indeed first. The change needs to be done in the benchmark code also. I have submitted a PR previously for benchmark, I'll make the change in all the schelling models in this repo and the maine mesa repo
examples/caching_and_replay/model.py
Outdated
| # its contents. (coord_iter) | ||
| for cell in self.grid.coord_iter(): | ||
| x, y = cell[1] | ||
| for _cont, pos in self.grid.coord_iter(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused loop variable should be named just _.
examples/caching_and_replay/run.py
Outdated
| Display an informational text about caching and the status of the cache file (existing versus not existing) | ||
| """ | ||
| cache_file = Path("./my_cache_file_path.cache") | ||
| cache_file = Path("./" + model_params["cache_file_path"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ./ should be part of model_params["cache_file_path"], given that it is a path instead of a filename.
|
Other than the comments, everything else LGTM. |
Changes Made:
Discussion Topic:
While it is possible to now replay models, I feel there are still some things that can be improved,