-
Notifications
You must be signed in to change notification settings - Fork 892
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
Generalize CellAgent #2292
Generalize CellAgent #2292
Conversation
4f7db3d
to
ec36c08
Compare
for more information, see https://pre-commit.ci
Triggered by the discussion in #2286, I have also been thinking about this and the idea of using some kind of mixin with specific movement methods had occurred to me as well. I thus like the idea of using composition / mixin a lot for this. It will raise some issues however regarding the use of super as discussed here. I had not thought of the protocol idea but again, yes I agree. |
Last year I started on a thesis proposal on this topic. At that time I couldn't find an angle that created a fit with my masters and potential supervisors, however I still think that something like this could be very interesting. I'm a bit tired today, but I will dive into this PR tomorrow with a hopefully sharp mind. |
Ah, yes that was the discussion, thanks for digging that up. And yes, this was basically your idea, so some credit goes to you! |
No worries, it isn't that novel. It might just contain a few ideas I had at the time that still might be useful. Also had a few thoughts jotted down here. It make sense we have some specialized agents for us that work with the components we ship in Mesa, in this case the spaces are the most obvious. |
def move(self: DiscreteSpaceAgent[Cell], direction: str, distance: int = 1): | ||
match direction: | ||
case "N" | "North" | "Up": | ||
self.move_relative((-1, 0), distance) | ||
case "S" | "South" | "Down": | ||
self.move_relative((1, 0), distance) | ||
case "E" | "East" | "Right": | ||
self.move_relative((0, 1), distance) | ||
case "W" | "West" | "Left": | ||
self.move_relative((0, -1), distance) | ||
case "NE" | "NorthEast" | "UpRight": | ||
self.move_relative((-1, 1), distance) | ||
case "NW" | "NorthWest" | "UpLeft": | ||
self.move_relative((-1, -1), distance) | ||
case "SE" | "SouthEast" | "DownRight": | ||
self.move_relative((1, 1), distance) | ||
case "SW" | "SouthWest" | "DownLeft": | ||
self.move_relative((1, -1), distance) | ||
case _: | ||
raise ValueError(f"Invalid direction: {direction}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Great use of pattern matching!
- Should we also allow lower case? If so, we can do
direction = direction.lower()
and then match against the lower case strings for example. - This now allows "queen" (chess) moves. Do we also want to allow knights and maybe even more? (south-south west, etc.)
- Do we want to allow a certain amount of degrees? 0 is up, 90 is east, 135 is south east, etc.
- Do we want to link this to the type of grid in any way? Because some grids (currently) have neighbours cells that are in different directions. Maybe add a filter "only_if_neighbour" or something better named than that.
I think none of these are blocking for this PR, just a few thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Thanks!
- Yes, they should also work with lowercase directions I think.
- This is supposed to be used on a regular grid, where each cell has 8 neighbboring cells it touches. So these are the base movements, everything else can be composed of these. This is of course a bit arbitrary, technically 4 base movements are sufficient (linked to von Neumann vs Moore). But I think its a convenient set
- I think degrees deserve a separate implementation. Actually my first implementation was heading more towards degrees, where each agent has a "heading" and movement is implemented in terms of forward/backward/turnLeft/turnRight. I think this kind of movement would also be nice!
- Well the class is specifically called Grid2DMovingAgent to address this. It only works with 2DGrids, not necessarily with higher dimensions or other subclasses of
DiscreteSpace
Obviously I'm in favor of this, since I wrote an whole proposal about this. One thing I would do different is the naming: One of my more coherent thoughts on this I wrote down here:
(note that So, in this case you're already almost fully adhering to my take on this: A single class To apply this way on this specific problem, you could have a few independent Behaviors (each a class with an single method): class MoveRelative2D:
def move_relative_2d(self: DiscreteSpaceAgent[Cell], direction: str, distance: int = 1):
...
class InView:
def in_view(self: DiscreteSpaceAgent[Cell], distance: float, field_of_view: float):
...
# From multiple single behaviors, you can make a behavior collection
class GridBehaviors(MoveRelative2D, InView)
... Now, you can either extend your Agent with a single behavior class, or with multiple. class MyMovingAgent(Agent, MoveRelative2D)
...
class MyGridAgent(Agent, GridBehaviors)
... Of course there many things. Especially naming of states is crucial to get this working properly. For example,
Currently, there's a strict separation between functions (no states as input and output, only arguments) and behaviors (only state inputs and updates). There are probably things in between, for example I think these are most of my implementation thoughts on this topic, this was about as far as I thought it through last year. Very curious where we take it from here! |
While I like the general idea of composition over inheritance, we have to be a bit carefull in how to do this. In the examples above we are effectivley using multiple inheritance for composition. This can be fine as longs as we make sure that any of the classes from which we are inheriting are mixin classes:
From this perspective, using multiple inheritance for movement can become tricky. For example, If instead, we want to rely on instance attributes for these various classes, you expose yourself to the mess that is multiple inheritance. This means potentially very fragile code, complex object |
In my vision Mixin behaviors can require certain Agent, other Agent or Model (or even Space) states to be present (and if not, throw and error), like "The MoveRelative2D behavior requires that the Agent has an But yeah, to do this properly it requires both practices, documentation and continuous checking. It's non-trivial. |
This is where the |
I think that really is crucial and so far I find it Incredible hard. But I don't agree that behaviours need to be restricted to single methods with the same name. That seems a bit too strict and fine-granular. I think its easy to imagine behaviours that are closely related and would feel more natural to be grouped. Also, this would lead to awkward names. Take again the |
Good point, I think I agree. For movement specifically, you would want to check against which space/grid you're working. I generally use property for attributes that are static during a model run, and state for attributes that might change. In Python both are of course just an attribute of a class and there is no implementation difference (I don't mean the Python |
A protocol in my understanding is just a specification of an interface that some object must meet to be considered of a particular type. It is richter than what I recall from java interfaces which only contained methods. So yes, we can use protocols to specify which attributes must be there. However, a protocol is only an interface definition, while mixins can also be used to add implementations to a class. So, to continue on the various move examples, yes we can define protocols for movment in DiscreteSpace, Grid, and ContinousSpace. This would be a major improvement on the current situation and help in resolving e.g. #2149, #2286, and this PR. Just as a quick though experiment, you could built up a protocol hierarchy along the lines outlined below, drawing on these PRs. A key unresolved issue is the mapping between cells and their "coordinates" in case of from typing import Protocol, TypeVar
from collections import namedtuple
Position = namedtuple("Position", "x y")
T = TypeVar("T", bound="Cell"|"Position")
class AgentMovement(Protocol):
position: T
def move_to(self, position) -> None: ...
class DiscreteMovement(AgentMovement):
class GridMovement(DiscreteMovement):
def move_relative(self, direction, distance:int =1) -> None: ...
class ContinousMovement(AgentMovement):
heading: float
def turn(self, degrees:float) -> None: ...
def turn_and_move(self, degrees:float, distance:float) -> None: ... But, ideally, we should also provide classes that provide default implementations for these space movement protocols. And here the question becomes whether we want to have some complex class hierarchy or whether it is possible to encapsulate the movement behavior in some way that allows for composition over inheritance. As soon as a protocol specifies attributes it becomes messy to encapsulate the protocol in a single class that can just be added via multiple inheritance. Put differently, do we want to have different base Agent classes for different spaces? Do we want the user to pick the appropriate base Agent class to use depending on the space they are using? Or do we want to provide components that users can add to their own agent depending on what they need? At the moment, a user can just subclass the generic agent class and be done with it. It seems unlikely that this simple case can persist if we want to provide default movement methods for different kinds of spaces. But I am unsure what is the next best solution. To be clear, I am all in favour of pursuing this PR, just trying to hash out more clearly what we exactly need and how different design directions might result in specific new problems/issues that need to be resolved. |
I don't think it makes a lot of sense to build a whole protocol tree and concrete classes for each specific space we can think of. In practice I believe both will become unwieldly. So I wouldn't go overboard with anything. But I would favor establishing a solid basis on which we can build upon. I think our current DiscreteSpace implementation is powerful enough to handle almost all spaces I can think of, with the exception of Continuous spaces. I think this is definitely a good enough basis and I don't think we nencesarily need to already think about API equality with continous spaces. So for the
Everything additional should be able to be implemented as mixins. This idea of making it more dependent on the cells connections may hint towards the previous idea of making them "named" via a dictionary with connection names. Which brings me to yet another idea of also having the coordinates of each cell "named". That is instead of coordinates like |
I agree that a full hierarchy is probably overkill. It was more of a quick exercise, partly for myself, to see what we would need. While doing this, I also considered the named connections idea in line with your suggestion. If connections are in some way named, I am not sure I fully understand the idea of named coordinates. Would this interact with I am less sure about e.g., a network, which currently just uses single numbers. |
Just to clarify, sorry about mentioning named coordinates. I'll bring up the idea somewhere else, it's not related to the connections at all, completely separate issue |
I'm starting to find SOTA LLMs slowly becoming kind of useful as resource for these discussions (if you ask the right questions). Below (in the details) 6 possible directions that might be useful, Dependency Injection is a technique I didn't consider before, I'm going to think a bit about that. 1. Embrace Composition Through DelegationInstead of using multiple inheritance or mixins to add movement behaviors to agents, consider using composition through delegation. This involves creating separate behavior classes (e.g., Advantages:
Example: class MovementBehavior:
def __init__(self, agent, space):
self.agent = agent
self.space = space
def move(self, direction, distance=1):
# Movement logic using self.agent and self.space
class MyAgent(Agent):
def __init__(self, unique_id, model, space):
super().__init__(unique_id, model)
self.movement = MovementBehavior(self, space) Implications:
2. Utilize the Strategy Design PatternThe Strategy pattern allows defining a family of algorithms (behaviors), encapsulating each one, and making them interchangeable. This pattern can be applied to movement behaviors. Advantages:
Example: class MovementStrategy(ABC):
@abstractmethod
def move(self):
pass
class GridMovementStrategy(MovementStrategy):
def __init__(self, agent, space):
self.agent = agent
self.space = space
def move(self):
# Implement grid movement
class ContinuousMovementStrategy(MovementStrategy):
def __init__(self, agent, space):
self.agent = agent
self.space = space
def move(self):
# Implement continuous movement
class MyAgent(Agent):
def __init__(self, unique_id, model, movement_strategy):
super().__init__(unique_id, model)
self.movement_strategy = movement_strategy
def step(self):
self.movement_strategy.move() Implications:
3. Define Clear Interfaces with Abstract Base Classes (ABCs)While Protocols are useful for static typing, using Abstract Base Classes (ABCs) can provide a concrete way to enforce the implementation of required methods and properties, especially when combined with composition. Advantages:
Example: from abc import ABC, abstractmethod
class MovableAgent(ABC):
@property
@abstractmethod
def position(self):
pass
@abstractmethod
def move_to(self, position):
pass
class MyAgent(Agent, MovableAgent):
def __init__(self, unique_id, model):
super().__init__(unique_id, model)
self._position = None
@property
def position(self):
return self._position
def move_to(self, position):
self._position = position Implications:
4. Leverage Dependency Injection for Space AwarenessInstead of having agents directly access the space or model, consider injecting dependencies such as the space or movement behaviors into agents. This reduces coupling and enhances testability. Advantages:
Example: class MyAgent(Agent):
def __init__(self, unique_id, movement_behavior):
super().__init__(unique_id)
self.movement_behavior = movement_behavior
def step(self):
self.movement_behavior.move() 5. Consider Event-Driven MovementShift from agents invoking movement methods directly to an event-driven approach, where agents emit movement intents, and a controller or the space itself handles the movement. Advantages:
Example: class MovementEvent:
def __init__(self, agent, direction):
self.agent = agent
self.direction = direction
class Space:
def process_movement_event(self, event):
# Handle movement, check for collisions, enforce rules
class MyAgent(Agent):
def step(self):
event = MovementEvent(self, direction='N')
self.model.space.process_movement_event(event) Implications:
6. Address Attribute Requirements Through InterfacesTo handle attributes like Advantages:
Example: class Positionable(Protocol):
position: Any
class MovementBehavior:
def __init__(self, agent: Positionable):
self.agent = agent
def move(self, delta):
# Update self.agent.position In the end I would really like some time (in the order of weeks) to figure this problem out properly. There's a fair chance that there's a workable solution that could have huge impact on how we develop (components of) Mesa models. Let's see if I ever get the chance to that. |
@EwoutH thanks for the overview. I think it covers virtually all options I can think of. The main drawback of 1-4, which are all forms of composition, is that the resulting API appears clumsy to me. In all cases, you must do Option 5 is effectively what is used in the legacy spaces. Conceptually, I don't like it because movement, in my view, is part of the behavior of the agent. So, having to do My own preference at the moment is some combination of a Protocol, base classes with movement behavior for discrete and continuous spaces, and then probably 3 agent classes: Alternatively, I would also be fine with |
Thanks both of you for the input. I fully agree with @quaquel. Its exactly how I see it and I am too unsure about whether we should subclass Agent or use multiple inheritance. My worries with Also we have to consider how we want to add more behaviours to agents. I think mixin classes (i.e. without Also with multiple inheritance we could still later abstract away all the behaviours into usable Agents, for example # namespace mesa.agents.grid2d
class MovableAgent(Agent, DiscreteSpaceAgent, Grid2DMovingAgent, AgentWithHeading, ...):
...
# User Model
class MyAgent(MovableAgent):
.... That is to say users can just subclass from 1 Agent, but if they need more granular options, they can start to learn about multiple inheritance and mixin classes |
I like this direction. Have meaningful defaults for a novice user, and flexibility to move beyond this. And the name The main question then becomes whether we can design the movement methods in a way that does not rely on unique attributes (to ensure the mixins don't need an To be fair, I haven't looked at ContinousSpace in a while, but I think this space could benefit from some redesign. For example, neighborhood search in contiuous spaces might be faster if the space internally uses a KD -tree. While for visualization, it would be convenient if you could just get a numpy array with the coordinates of all agents. |
That's exactly what I envisioned in #2292 (comment), so I think we fully align on this!
Not sure. Many models don't use a grid at all, or use a networkgrid, or have some own custom implementations for this stuff. But I agree it should be named logically and unambiguous.
I find this a difficult one. Because:
So I get where the current implementation came from, all the information is in the space. Somehow if we want to do this in the Agent, there should be an infromation transfer from the Space to the Agent. Few options (for the information transfer):
The biggest problem however is that some space will allow some movements, and other's don't. For example, a movement with an heading can be only allowed in continuous spaces. We might be able to catch that with documentation and warnings, but it might still cause some confusion and bloat if we all put that into one class. |
But the new cell space was defined specifically to close that information gap! Space now only define: Which cells exist and how they are connected. Movement itself is implemented via the cells and they also have the Information about capacity, neighborhood, etc. So I don't think we need to have a reference to space by default. You do need that information for example if you want to query empty cells, or move to a random cell. But I think then the space is a kind of model information so accessing it through
Exactly, this gets too bloated quickly I believe. But lets not overcomplicate things. I think we only have 3 states
1 has no conflicts with the others, 2 and 3 might conflict. So if we think agents being part of a discrete space and continuous space is a common (or at least not unreasonable) use case we should take care to have no overlap between both, so agents can be both DiscreteAgents and ContinuousAgents (for example having |
I am still wondering whether it would be possible to use a common interface on a position in a Next, you might enrich this minimum form of movement further, but most of that would be built on top of this common interface. |
Thanks for linking me to this interesting discussion @EwoutH. I'm still reading through the comments and trying to understand what's happening here. Looks like we're trying to adopt a type system in Mesa? Is this similar to traits in Rust (not to be confused with our
This has been explored in #1619. Mesa-Geo uses |
Yes! That is exactly what we are talking about, but then trying to make it work within Python's OO multiple inheritance structure. |
From #2333 (comment)
I added a Patch agent in b0e95a0 based on the things happening in this PR. straight-foward! The position can be set if its is None before, but after that you cant move it somewhere else |
for more information, see https://pre-commit.ci
… into cell-space-agents
I have added tests for the new stuff, which is now fully covered. I also changed a few minor things. I also added a custom # hard ref to patch prevents gc on patch.remove()
patch = Patch(model)
patch.cell = cell1
patch.remove() # if this sets self._mesa_cell to None we could do
patch.cell = cell2 |
Thanks a lot for doing this! Really helps moving this forward. Regarding Grid2DMovingAgent, I am honestly still not 100% sure on how to proceed with this one. But my intention was actually to first leave it only as a mixin class. Thus it was misnamed "agent" and should have ended in -Movement. But maybe we can have both or just the agent. I honestly don't know which class to "mix in" if not CellAgent. Regarding Patch. Not sure what's the current status with the fixme. Does this now prevent movement-by-remove already or what do we need to fix? |
Ok, let's leave it like it is for now. It at least showcases the possibilities if users need e.g., a 3D Grid movement agent.
The current version works correctly, the fixme was more reminder for myself. So you cannot move-by-remove. In my view, remove should really mean: remove agent from model and clean up any hard refs so it can be garbage collected. I might have gone overly defensive with the implementation of |
got it now, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few minor comments, non blocking. Let's move this forward and build some example models with it!
Breaking it can be done always, if we find that useful after some iterating.
Conceptually no, implementation yes ;) Means we're asking the same questions, great minds think alike? 😇
I think both are fine, I like the latter sounds like the most proper / modular way to do it, so I like that a tiny bit more.
Smart! |
Co-authored-by: Ewout ter Hoeven <[email protected]>
I prematurely added the suggestion to rename Patch to FixedAgent. Don't think we need the netlogo terminology here. But I forgot that this now broke the tests and am running out of time to fix this I hereby grant you to do whatever you want with this PR in the next 10 days. Probably just fix the tests and merge it to move forward? |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
No worries, we got you. Updated the tests and also directly used the I'm good to merge, @quaquel? |
Performance benchmarks:
|
This PR introduces a new approach to agent design in Mesa using the Cell space, focusing on composition over inheritance and using Protocols for flexible type hinting. It includes new agent types:
CellAgent
,FixedAgent
, andGrid2DMovingAgent
, which provide specialized behavior for discrete space simulations.Motive
The main goals of this PR are:
These changes aim to make Mesa more adaptable to various modeling scenarios and to improve type checking and IDE support.
Implementation
CellAgent
is now a a composition of Agent and 2 mixins:HasCell
andBasicMovement
Protocol
forDiscreteSpaceAgent
to allow for flexible type checking.Grid2DMovingAgent
with specialized movement methods for 2D grids. At the moment this is implemented as a subclass ofCellAgent
. As discussed below, we might want to split this in the future into a separate Grid2DMovement mixin andGrid2DMovingAgent
, which would be a composition ofCellAgent
andGrid2DMovement
FixedAgent
for agents that don't move (similar to NetLogo's patches).Cell
class to useCellCollection
for neighborhoods.Key changes:
CellAgent
now includesBasicMovement
mixin.Grid2DMovingAgent
provides convenient directional movement methods.FixedAgent
prevents movement after initial placement.Usage Examples
Additional Notes
FixedAgent.remove()
prevents movement by removal, ensuring fixed agents remain in place.Original PR description
This is a bit of an exploration on some different design principles, as well as adding a new specialized 2D Grid Agent that has a convenience movement functions.There are 2 design decisions that to my knowledge we haven't used in mesa yet:
Protocol
to allow "static duck typing" (or structural subtyping)Lets first discuss 1)
In this implementation
CellAgent
is no longer a subtype ofAgent
. This is based on some previous discussions about the type hierachy of agents. Without being a subtype we circumvent problems that might arise by the use of more agent types, with different behaviours. In practice, if you want to add cell specific behaviour you define your agent like so:So you would inherit both from
Agent
as well asCellAgent
. On the other hand, if you don't incorporate the new discrete spaces, you don't need to addCellAgent
s. And if you want to useDiscreteSpace
s for other purposes from the standard model way (or for experimentations with the space), you don't need the basicAgent
functionality (i.e. mostly associating a model with the agent).This PR also includes a
Grid2DMovingAgent
that is even more specialized in its movement as an additional mixin.This class has an interesting type signature:
Here, self is a type from another class:
The usage of
Protocol
here allows static duck typing (ignoring the generic T for the moment). That means themove
method of Grid2DMovingAgent requires an instance that satisfies the needs ofDiscreteSpaceAgent
. It could be a CellAgent, but it could also be an instance of any other class, as long as it includes thecell
andspace
attributes and themove_to, move_relative
methods. So with Protocols we can design our apis much more flexible. For example currently in the cell_space we have some typesCellAgent
. But most of the time the parameters don't actually have to be a subtype of CellAgent, they just need to have acell
attribute. This can be expressed with Protocols.So, lets discuss these design principles and if we should adopt them.