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

Improving RasterLayer #120

Closed
wang-boyu opened this issue Nov 6, 2022 · 4 comments
Closed

Improving RasterLayer #120

wang-boyu opened this issue Nov 6, 2022 · 4 comments
Labels
enhancement Release notes label
Milestone

Comments

@wang-boyu
Copy link
Member

What's the problem this feature will solve?

As mentioned in the docstrings, some methods in RasterLayer are copied from mesa.space.Grid, including:

__getitem__
__iter__
coord_iter
iter_neighborhood
get_neighborhood
iter_neighbors
get_neighbors  # copied and renamed to `get_neighboring_cells`
out_of_bounds  # copied into `RasterBase`
iter_cell_list_contents
get_cell_list_contents

Recently mesa.space.Grid has received some improvements (refactoring, performance improvements, and so on) but these are not reflected in RasterLayer.

Describe the solution you'd like

Wait until some pending PRs get merged and copy these changes over, until we figure out a better way to incorporate these changes automatically (like mentioned at #75 (comment)).

@wang-boyu wang-boyu added this to the v0.5.0 milestone Nov 6, 2022
@wang-boyu wang-boyu added the enhancement Release notes label label Nov 6, 2022
@wang-boyu wang-boyu removed this from the v0.5.0 milestone Nov 6, 2022
@rht
Copy link
Contributor

rht commented Nov 10, 2022

I haven't been able to find a class name that encompasses those methods, while excluding

torus_adj
neighbor_iter
move_agent
place_agent
_place_agent
remove_agent
is_cell_empty
move_to_empty
find_empty
exists_empty_cells

What would be the consequence for RasterLayer to have those methods? Maybe the *_agent methods can be replaced with methods that raise NotImplementedError?

@wang-boyu
Copy link
Member Author

wang-boyu commented Nov 11, 2022

The main difference between RasterLayer and Gridare:

  1. The GridContent in Grid is Cell in RasterLayer. This is fine since GridContent = Union[Agent, None] and Cell inherits Agent.
  2. In Grid there is self.grid: list[list[GridContent]] whereas in RasterLayer we have self.cells: list[list[Cell]]. This is a bit troublesome because self.grid is effectively renamed to self.cells.

Another issue is that we do not manipulate agents using RasterLayer directly. Instead, adding/updating/removing agents methods are defined at the GeoSpace level. It might be confusing if we have these methods also defined within RasterLayer and raise NotImplementedError.

I couldn't think of any easier ways to get necessary methods from Grid while avoiding the rest. Was hoping that changes wouldn't happen very often.

On the other hand, I don't want to alter the design of Mesa due to Mesa-Geo either.

@wang-boyu wang-boyu removed the backlog label Nov 24, 2022
@wang-boyu wang-boyu added this to the Backlog milestone Nov 24, 2022
@wang-boyu
Copy link
Member Author

Mesa now has an experimental PropertyLayer and could be an alternative implementation for our RasterLayer: projectmesa/mesa#1898

@wang-boyu
Copy link
Member Author

Replaced by #201. Closing this issue now.

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

No branches or pull requests

2 participants