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

move_agent_to_neighborhood #1953

Closed
rht opened this issue Jan 11, 2024 · 15 comments
Closed

move_agent_to_neighborhood #1953

rht opened this issue Jan 11, 2024 · 15 comments

Comments

@rht
Copy link
Contributor

rht commented Jan 11, 2024

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

@quaquel
Copy link
Member

quaquel commented Jan 11, 2024

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)

@Corvince
Copy link
Contributor

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)

@quaquel
Copy link
Member

quaquel commented Jan 13, 2024

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.

@rht
Copy link
Contributor Author

rht commented Jan 14, 2024

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?

@quaquel
Copy link
Member

quaquel commented Jan 14, 2024

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 #1900 (comment) 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.

@rht
Copy link
Contributor Author

rht commented Jan 14, 2024

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))?

@quaquel
Copy link
Member

quaquel commented Jan 15, 2024

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 Proposal: Formal neighborhood definition for grids #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.

@rht
Copy link
Contributor Author

rht commented Jan 15, 2024

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.

@quaquel
Copy link
Member

quaquel commented Jan 15, 2024

  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.

@Corvince
Copy link
Contributor

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.

@rht
Copy link
Contributor Author

rht commented Jan 16, 2024

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 writingintuitiveness (of the code).

@rht
Copy link
Contributor Author

rht commented Jan 16, 2024

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);

@quaquel
Copy link
Member

quaquel commented Jan 16, 2024

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 writingintuitiveness (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).

@rht
Copy link
Contributor Author

rht commented Jan 16, 2024

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:

to move-to-random-empty-neighbor
  let empty-neighbors neighbors with [not any? turtles-here]
  if any? empty-neighbors [
    move-to one-of empty-neighbors
  ]
end
ask my-turtle [move-to-random-empty-neighbor]

EwoutH pushed a commit that referenced this issue Feb 27, 2024
## Summary
This PR introduces an alteranative implementation for discrete spaces. This implementation centers on the explicit inclusion of a Cell class. Agents can occupy cells. Cells have connections, specifying their neighbors. The resulting classes offer a cell centric API where agents interact with a cell, and query the cell for its neighbors. 

To capture a collection of cells, and their content (_i.e._, Agents), this PR adds a new CellCollection class. This is an immutable collection of cells with convenient attribute accessors to the cells, or their agents. 

This PR also includes a CellAgent class which extends the default Agent class by adding a `move_to` method that works in conjunction with the new discrete spaces. 

From a performance point of view, the current code is a bit slower in building the grid and cell data structure, but in most use cases this increase in time for model initialization will be more than offset by the faster retrieval of neighboring cells and the agents that occupy them.

## Motive
The PR emerged out of various experiments aimed at improving the performance of the current discrete space code. Moreover, it turned out that a cell centric API resolved various open issues (_e.g._, #1900, #1903, #1953). 

## Implementation
The key idea is to have Cells with connections, and using this to generate neighborhoods for a given radius. So all discrete space classes are in essence a [linked data structure](https://en.wikipedia.org/wiki/Linked_data_structure).

The cell centric API idea is used to implement 4 key discrete space classes: OrthogonalMooreGrid, OrthogonalVonNeumannGrid (alternative for SingleGrid and MultiGrid, and moore and von Neumann neighborhood) , HexGrid (alternative for SingleHexGrid and MultiHexGrid), and Network (alternative for NetworkGrid). Cells have a capacity, so there is no longer a need for seperating Single and Multi grids. Moore and von Neumann reflect different neighborhood connections and so are now implemented as seperate classes. 

---------

Co-authored-by: Jan Kwakkel <j.h.kwakkel@tudelft.nl>
quaquel added a commit to quaquel/mesa that referenced this issue Apr 9, 2024
## Summary
This PR introduces an alteranative implementation for discrete spaces. This implementation centers on the explicit inclusion of a Cell class. Agents can occupy cells. Cells have connections, specifying their neighbors. The resulting classes offer a cell centric API where agents interact with a cell, and query the cell for its neighbors. 

To capture a collection of cells, and their content (_i.e._, Agents), this PR adds a new CellCollection class. This is an immutable collection of cells with convenient attribute accessors to the cells, or their agents. 

This PR also includes a CellAgent class which extends the default Agent class by adding a `move_to` method that works in conjunction with the new discrete spaces. 

From a performance point of view, the current code is a bit slower in building the grid and cell data structure, but in most use cases this increase in time for model initialization will be more than offset by the faster retrieval of neighboring cells and the agents that occupy them.

## Motive
The PR emerged out of various experiments aimed at improving the performance of the current discrete space code. Moreover, it turned out that a cell centric API resolved various open issues (_e.g._, projectmesa#1900, projectmesa#1903, projectmesa#1953). 

## Implementation
The key idea is to have Cells with connections, and using this to generate neighborhoods for a given radius. So all discrete space classes are in essence a [linked data structure](https://en.wikipedia.org/wiki/Linked_data_structure).

The cell centric API idea is used to implement 4 key discrete space classes: OrthogonalMooreGrid, OrthogonalVonNeumannGrid (alternative for SingleGrid and MultiGrid, and moore and von Neumann neighborhood) , HexGrid (alternative for SingleHexGrid and MultiHexGrid), and Network (alternative for NetworkGrid). Cells have a capacity, so there is no longer a need for seperating Single and Multi grids. Moore and von Neumann reflect different neighborhood connections and so are now implemented as seperate classes. 

---------

Co-authored-by: Jan Kwakkel <j.h.kwakkel@tudelft.nl>
@quaquel
Copy link
Member

quaquel commented Oct 30, 2024

With the aim to stabilize cell_space in 3.1 and old-style grids becoming maintenance only in 3.0 this is resolved.

@quaquel quaquel closed this as completed Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants