mesa icon indicating copy to clipboard operation
mesa copied to clipboard

Grid: Calling `place_agent` twice in different positions causes an agent to exist in two positions in the grid

Open rht opened this issue 3 years ago • 6 comments

Describe the bug

Context: https://github.com/projectmesa/mesa/pull/1508#discussion_r1014769506 Currently, calling place_agent twice or more in a row without calling remove_agent beforehand causes the agent to exist in several locations.

Expected behavior

An error should be raised if an agent's pos attribute is not None, when calling place_agent. A stronger guarantee would be to check the entire grid for the existence of the agent, but this is very costly to do, unless there is a dict attribute for the grid to track the agents existence.

To Reproduce

import mesa

a = mesa.Agent(0, None)
grid = mesa.space.SingleGrid(3, 3, False)
grid.place_agent(a, (1, 1))
print("a.pos", a.pos)
grid.place_agent(a, (0, 0))
print("a.pos", a.pos)
for e in grid.coord_iter():
    print(e)

Output

a.pos (1, 1)
a.pos (0, 0)
(<mesa.agent.Agent object at 0x7ffa9311a9e0>, 0, 0)
(None, 0, 1)
(None, 0, 2)
(None, 1, 0)
(<mesa.agent.Agent object at 0x7ffa9311a9e0>, 1, 1)
(None, 1, 2)
(None, 2, 0)
(None, 2, 1)
(None, 2, 2)

rht avatar Nov 11 '22 10:11 rht

I think this is really linked with discussion #1513 because if we had a dict inside grid it would be more difficult for the user to change the pos (we can create a method which returns the agent on the position requested so that the user has no access to the dict easily unless he goes to read the source code, then the pos is just updated internally and so agent.pos is not None would be sufficient)

Tortar avatar Nov 11 '22 11:11 Tortar

I wouldn't consider this a bug at all. In Agent-Based Modelling we should not assume too much about what an agent is and what a grid represents, it could very well be an abstract space, where an agent can exist in two places at the same time just fine.

But even in a concrete space setting it should be possible. Imagine building a maze in mesa. There is no reason why one should not have a single "Wall" agent that exists on multiple cells.

What could be nice is a warning that is displayed when an agent already exists. This would help catch some bugs where agents really shouldn't exist in more than one place, but would still allow to do so if needed by the model.

Corvince avatar Nov 11 '22 12:11 Corvince

The wall/maze use case makes sense. But there would still be warning messages. Either these are suppressed with filter warning, or there is a flag parameter in the Grid initialization. I'm not sure if I would classify it as something along the line of NaNs in Numpy.

rht avatar Nov 11 '22 15:11 rht

yet agent.pos is updated so if place_agent is called twice the first pos is lost right now

Tortar avatar Nov 11 '22 17:11 Tortar

yet agent.pos is updated so if place_agent is called twice the first pos is lost right now

Well I would say it depends on the user wether the pos is lost. From a pure software development standpoint I would say the grid shouldn't be responsible for the agents position. It should have a single responsibility and that should be the grid/space itself.

I would argue updating the pos attribute is done purely out if convenience, but this might also be surprising for some users.

Imho the API would be cleaner if it would just return the position, so the user would typically do something like

agent.pos = grid.place_agent(agent, pos)

But this is a design decision. I guess I am trying to say we shouldn't go over board with managing things that are too far outside the scope of the grid. We have to strike a balance between doing things for convenience and restricting user flexibility (because ultimately we don't know how (or if) users want to save the position of their agents)

Corvince avatar Nov 13 '22 09:11 Corvince

We need to optimize the API for convenience of the most common use cases. In most cases, having an agent to be in 2 places is problematic. As a user, when prototyping, I might assume place_agent would handle the removal of the agent when this is called. A warning message mitigates this, and will likely save people's debugging time. We don't want a bug due to human error, along the line of #1509 to happen again (and this bug has been hiding in the Schelling example for a long time).

Regarding with the dict: the pos attribute is often needed to be shown in the portrayal, accessed from the agent, regardless if there is a dict[agent_key, agent_pos] stored in the grid.

rht avatar Nov 14 '22 10:11 rht