mesa icon indicating copy to clipboard operation
mesa copied to clipboard

move_agent_to_neighborhood

Open rht opened this issue 2 years ago • 14 comments

What's the problem this feature will solve? Currently, there are several examples [1] where the agents are supposed to move to a random location within the neighborhood. This is implemented with a rather verbose class, RandomWalker, .

next_moves = self.model.grid.get_neighborhood(self.pos, self.moore, True)
next_move = self.random.choice(next_moves)
# Now move:
self.model.grid.move_agent(self, next_move)

If you look at https://github.com/JuliaDynamics/ABMFrameworksComparison/tree/main/WolfSheep/Mesa, the random_walk.py is one of the reason why Mesa has a long LOC.

Describe the solution you'd like

It would be great if there is a method move_agent_to_neighborhood that encapsulates the random walk procedure defined in the previous section.

[1] bank reserve, Boltzmann wealth model, wolf sheep

rht avatar Jan 11 '24 02:01 rht

This might or might not be a possible direction, but one I am curious to explore if I have a bit of time:

In various contexts, adding a dedicated "patch" or "grid cell" class to MESA has been suggested. This would be a way to give agent-like behavior to the grid (e.g., grass regrowth dynamics). If there is a dedicated grid cell class, you could specify within it, its neighbors (e.g., left, right, top bottom for a Moore neighborhood). You can even do larger neighborhood sizes on top of this by recursively querying these attributes. Moreover, you can eliminate boundary checking when dealing with non-toroidal spaces.

Long story short, if each grid cell class has a collection of neighbors, the random walk becomes trivial:

neighbors = self.my_gridcell.neighbors(size=1)
next_move = self.random.choice(neighbors)

quaquel avatar Jan 11 '24 07:01 quaquel

If you look at https://github.com/JuliaDynamics/ABMFrameworksComparison/tree/main/WolfSheep/Mesa, the random_walk.py is one of the reason why Mesa has a long LOC.

In my opinion its a bit idiotic to rate LOC, but if you are concerned with respect to the framework comparison you could submit a PR where it is condensed in a single line:

self.model.grid.move_agent(self, self.random.choic(self.model.grid.get_neighborhood(self.pos, True, 1)))

And rermove the RandomWalker class. For me it shows how you can easily define a base class for your agents, but in a comparison that counts LOC it is probably too verbose.

(This is not to say move_agent_to_neighborhood would be an unnecessary addition. I have no opinion on whether it should be added or not)

Corvince avatar Jan 13 '24 20:01 Corvince

I grant you that LOC is a bit tricky to judge and should never be relied upon. Still, it is a proxy for how expressive the language/library is. It is one of the reasons I switched to Python from JAVA: a lot of stuff could be done much more easily.

quaquel avatar Jan 13 '24 21:01 quaquel

I agree with @quaquel in regarding LOC as a <100% proxy of expressivity. I'm definitely not seeking to rewrite it in a way that is unreadable & not idiomatic Mesa, like self.model.grid.move_agent(self, self.random.choic(self.model.grid.get_neighborhood(self.pos, True, 1))). @EwoutH how would move_agent_to_neighborhood and move_agent_to_empty_neighborhood be expressed in NetLogo?

rht avatar Jan 14 '24 14:01 rht

One observation from my side is that, at the moment, all calls are done via Grid / Space. As briefly discussed in #1900, it might be worth considering a way of expressing this through the individual grid cells instead of or as a complement to the current Grid API.

For example, @Corvince suggested in https://github.com/projectmesa/mesa/issues/1900#issuecomment-1890757831 self.cell.neighborhood.select(n=1).move_to(self). It reads nicely and is very similar to what we already have with the API of AgentSet.

quaquel avatar Jan 14 '24 15:01 quaquel

This issue seems to be a more suitable place to discuss about the API than #1900.

self.cell.neighborhood.select(n=1).move_to(self)

If I do cell.neighborhood.select(n=3), would adding a move_to(self) make sense? move_to() would have made more sense than move_to(self), given that for it being a subject-verb-object sentence, move_to(self) looks more like subject-verb-subject.

What about moving all the move actions definitions to agent class: agent.move_to(cell.neighborhood.select(n=1)), agent.move_to_one_of(cell.neighborhood.select(n=3))?

rht avatar Jan 14 '24 22:01 rht

The API language point is well taken. I broadly agree.

However, is moving in a space or grid part of the behavior of an Agent? Or does this behavior belong to a Space or GridCell class?

  1. not all ABMs I have made rely on spaces, so moving in space is not essential to the Agent. This could be solved by building a class hierarchy with Agent, and PositionalAgent (or something better named). There is also a link to the stale PR1354 where @rht already suggested subclassing Agent.
  2. You'll need the Space/Grid class to keep track of stuff like empties. Having GridCell classes might also be helpful for various reasons (see the ongoing discussion in #1900).

Just thinking this through in light also of the API of AgentSet you could do something like this

# within an agent
self.cell.neighborhood.shuffle().select(n=1).place_agent(self).

# shuffle to select only one might not be the most performant (to be tested)
self.cell.neighborhood.select_random().place_agent(self).

Note that both examples still rely on a dedicated GridCell or Position object. First, it is used to get the direct neighborhood. And next, this class contains the place_agent method. Second, shuffle, select, and/or select_random would belong to a PositionSet or GridCellSet class.

quaquel avatar Jan 15 '24 06:01 quaquel

not all ABMs I have made rely on spaces, so moving in space is not essential to the Agent.

I see your point regarding with moving being specific to space, but not placing move to be a method of Agent, would result in a longer method name, move_agent_to instead of move_to

.select_random() is definitely faster and consumes less memory (without having to create a copy of another shuffled neighborhood) than .shuffle().select(n=1).

Tangent to above points, but I come to a realization that the API place_agent is unnecessary. move_agent works just fine to place an agent for the first time to a grid. Or at least the underlying implementation can be made such that it just works, and we have a smaller API set.

rht avatar Jan 15 '24 10:01 rht

  1. For me, conceptual soundness and clarity trump concerns about the length of a method name (within reasonable limits clearly). In particular, with autocomplete being omnipresent these days, there is no practical difference from the speed of coding point of view between either method name.
  2. Shifting moving behavior from space-related classes to an Agent class is a profound conceptual API change. I would only do something big like that if there are very good reasons for it.
  3. We probably should add .select_random() to AgentSet as well. Still curious to do a quick timeit on it just to see how big of a difference it makes.
  4. Good point. It might indeed be possible to merge the two. A quick look, however, suggests it might be a bit tricky because place_agent differs across the different Grid classes.

quaquel avatar Jan 15 '24 16:01 quaquel

Agree with everything @quaquel . I especially like the idea of select_random(), because its a nice syntactic way to return a single element instead of a set, so we should definitely consider it for AgentSet as well.

Corvince avatar Jan 15 '24 20:01 Corvince

I find .select(how="random") to be more general, which allows .select(how="first") without having to define an extra method.

For me, conceptual soundness and clarity trump concerns about the length of a method name

Additionally, it reads more like a natural language, agent.move_to(cell.neighborhood.select_random()), and is surely easier for the user to recall, than agent.cell.neighborhood.select_random().move_agent_to(agent). And hence why I was asking @EwoutH how NetLogo does it, because they had already thought of this issue from the perspective of ~~ease of writing~~intuitiveness (of the code).

rht avatar Jan 16 '24 02:01 rht

This reminds me of Steve Yegge's old rant about Java's unnatural/bureaucratic API: https://steve-yegge.blogspot.com/2006/03/execution-in-kingdom-of-nouns.html, natural language pseudocode

get the garbage bag from under the sink
  carry it out to the garage
  dump it in the garbage can
  walk back inside
  wash your hands
  plop back down on the couch
  resume playing your video game (or whatever you were doing) 

compared with Java

For the lack of a nail,
    throw new HorseshoeNailNotFoundException("no nails!");

For the lack of a horseshoe,
    EquestrianDoctor.getLocalInstance().getHorseDispatcher().shoot();

For the lack of a horse,
    RidersGuild.getRiderNotificationSubscriberList().getBroadcaster().run(
      new BroadcastMessage(StableFactory.getNullHorseInstance()));

For the lack of a rider,
    MessageDeliverySubsystem.getLogger().logDeliveryFailure(
      MessageFactory.getAbstractMessageInstance(
        new MessageMedium(MessageType.VERBAL),
        new MessageTransport(MessageTransportType.MOUNTED_RIDER),
        new MessageSessionDestination(BattleManager.getRoutingInfo(
                                        BattleLocation.NEAREST))),
      MessageFailureReasonCode.UNKNOWN_RIDER_FAILURE);

rht avatar Jan 16 '24 02:01 rht

I find .select(how="random") to be more general, which allows .select(how="first") without having to define an extra method.

For me, conceptual soundness and clarity trump concerns about the length of a method name

Additionally, it reads more like a natural language, agent.move_to(cell.neighborhood.select_random()), and is surely easier for the user to recall, than agent.cell.neighborhood.select_random().move_agent_to(agent). And hence why I was asking @EwoutH how NetLogo does it, because they had already thought of this issue from the perspective of ~ease of writing~intuitiveness (of the code).

There are conceptual arguments for adding grid and continuous space movement behavior to subclasses of Agent. Fundamentally, movement is part of the behavior of an Agent, and the agent moves on or in a space.

My question is whether these arguments are strong enough to warrant a massive change in the basis structure of MESA. I am presently not convinced. I am partly unconvinced because we have not fully explored solutions within the current architecture.

I find .select(how="random") to be more general, which allows .select(how="first") without having to define an extra method.

I disagree. I prefer having a slightly larger set of methods that do a clearly defined thing over a few methods with a long list of arguments and keyword arguments that I can never remember. This is partly because of convenience; using autocomplete to scan the available methods quickly allows me to spot what I need quickly. In the case of many args and kwargs, I have to read the documentation every time I use the method. Moreover, the resulting code for a single method will be annoying to read with various ifs etc. (as evidenced by AgentSet.select).

quaquel avatar Jan 16 '24 06:01 quaquel

This is ChatGPT's answer for NetLogo's move_agent_to_neighborhood: ask my-turtle [move-to one-of neighbors], while for move_to_empty:

rht avatar Jan 16 '24 07:01 rht