mesa icon indicating copy to clipboard operation
mesa copied to clipboard

Issues with the Grid implementation

Open Corvince opened this issue 4 years ago • 2 comments

I have been poking around the grid classes for some time now and finally want to list all of the issues I have found so far (roughly going from minor to major).

  1. Class docstring doesn't mention the empties property
  2. Class docstring for methods provide a confusing order
  3. Docstring of move_to_empty is wrong for MultiGrids.
  4. position_agent docstring mentions swap_pos, but this function does not exist.
  5. We have a neighbor_iter(pos, moore) and an iter_neighbors(pos, moore, include_center=False, radius=1 function, which behave the same, except the former lacks some options
  6. SingleGrid provides a position_agent(agent, x="random", y="random") function, but there is nothing special about it (would work for Grid and MultiGrid as well).
  7. We allow accessing grid contents via grid[x][y] which looks like a normal attribute access, but actually just returns the inner list of the grid list of lists and access an item there. I think allowing grid[x, y] would be more intuitive, prevent the frequent unpacking of pos into x, y and be independent of the implementation (list of lists)
  8. iter_cell_list_contents for MultiGrid returns all the agents in a list of cells, but there is now way of knowing from which cell the individual agents yield from (because everything is chained together).
  9. providing iter_* and get_* methods may be confusing for people new to python and also in practice provide little additional benefits (I think all of the examples simply iterate over all values, thus I think always returning lists as is an easier solution).
  10. We currently have Grid, SingleGrid and MultiGrid. Let's imagine we have a grid with a single location (0,0) and try to place two agents (Agent A and Agent B) there. Initially we have the following situation: Grid: None, SingleGrid: None, Multigrid: {} (empty set) Now we can call grid.place_agent(AgentA, (0,0)) This is the content now: Grid: Agent A; SingleGrid: Agent A, MultiGrid: {Agent A} Again: grid.place_agent(AgentB, (0,0)) Now it gets weird: Grid: Agent B; SinlgeGrid: Agent A (with "Error: Not empty"); MultiGrid: {Agent A, Agent B} OR {Agent B, Agent A} I think only SingleGrid behaves in a meaningful way. Grid just overrides any content, but AgentA.pos will still refer to (0,0). MultiGrid provides an undefined order or agents, which could causes non-reproducible results.

This is my list for now. I will create a work-in-progress PR to solve most of the issues above and some additional goodies. This issue should be used to list additional oddities or discuss ideas for a better API.

Corvince avatar Mar 25 '20 10:03 Corvince

May I add another note?

The cells in a Grid do not have any attributes. I understand that this is by design, but I think that a common enough use case would be that the grid is a "landscape" where agents move. A good example is the Sugarscape model, where we have Sugar agent, which is fixed in position and has various attributes. I'm working on a land use model, and each cell is a parcel with its own attributes as well.

It would be nice to have a grid class that gives cells a special treatment, allowing adding arbitrary attributes while exempting them from moving around.

majdal avatar Jun 13 '20 16:06 majdal

I will suggest also to rethink the MultiGrid and the SingleGrid class in order to not depend in the Rectangular class Grid. Grid is sort of a base class of a rectangular grid. HexGrid is a subclass of it, but MultiGrid and SingleGrid are entirely base on Grid, the way these two classes were coded (probably before the HexGrid was coded) is not taking advantage of the Factory pattern that could be used to 'parametrize' these two classes.

hebertodelrio avatar Aug 18 '21 20:08 hebertodelrio