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

Generalize CellAgent #2292

Merged
merged 21 commits into from
Oct 4, 2024
Merged

Generalize CellAgent #2292

merged 21 commits into from
Oct 4, 2024

Conversation

Corvince
Copy link
Contributor

@Corvince Corvince commented Sep 12, 2024

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, and Grid2DMovingAgent, which provide specialized behavior for discrete space simulations.

Motive

The main goals of this PR are:

  1. To provide more flexibility in agent design by favoring composition over inheritance.
  2. To introduce "static duck typing" (or structural subtyping) using Protocols.
  3. To create specialized agent types for different space configurations without enforcing a rigid type hierarchy.

These changes aim to make Mesa more adaptable to various modeling scenarios and to improve type checking and IDE support.

Implementation

  1. CellAgent is now a a composition of Agent and 2 mixins: HasCell and BasicMovement
  2. Introduced Protocol for DiscreteSpaceAgent to allow for flexible type checking.
  3. Created Grid2DMovingAgent with specialized movement methods for 2D grids. At the moment this is implemented as a subclass of CellAgent. As discussed below, we might want to split this in the future into a separate Grid2DMovement mixin and Grid2DMovingAgent, which would be a composition of CellAgent and Grid2DMovement
  4. Implemented FixedAgent for agents that don't move (similar to NetLogo's patches).
  5. Updated the Cell class to use CellCollection for neighborhoods.
  6. Modified existing examples and tests to work with the new agent types.

Key changes:

  • CellAgent now includes BasicMovement mixin.
  • Grid2DMovingAgent provides convenient directional movement methods.
  • FixedAgent prevents movement after initial placement.

Usage Examples

# Creating a movable agent in a 2D grid
class MyAgent(Grid2DMovingAgent):
    def step(self):
        self.move("north", 2)  # Move 2 cells north

# Creating a fixed agent (like a patch)
class MyPatch(FixedAgent):
    def __init__(self, model):
        super().__init__(model)
        self.resource_level = 100

# Using a CellAgent without grid-specific movement
class MySimpleAgent(CellAgent):
    def step(self):
        new_cell = self.cell.get_neighbors().random_choice()
        self.move_to(new_cell)

Additional Notes

  • This PR builds upon the discussions in Move experimental cell spaces to normal #2286 and Encapsulate cell movement in properties #2333.
  • The implementation of FixedAgent.remove() prevents movement by removal, ensuring fixed agents remain in place.
  • Future work may include further refinement of the movement protocols and potentially adding support for continuous spaces.
  • The changes introduced here are designed to be backward compatible while providing new functionality for more complex agent behaviors.
Original PR descriptionThis 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:

  1. Favoring Composition over inheritance with regard to CellAgents
  2. Usage of a Protocol to allow "static duck typing" (or structural subtyping)

Lets first discuss 1)
In this implementation CellAgent is no longer a subtype of Agent. 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:

class MyAgent(Agent, CellAgent):
    ...

So you would inherit both from Agent as well as CellAgent. On the other hand, if you don't incorporate the new discrete spaces, you don't need to add CellAgents. And if you want to use DiscreteSpaces for other purposes from the standard model way (or for experimentations with the space), you don't need the basic Agent 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:

class Grid2DMovingAgent:
    def move(self: DiscreteSpaceAgent[Cell], direction: str, distance: int = 1):
        ...

Here, self is a type from another class:

class DiscreteSpaceAgent(Protocol[T]):
    cell: T | None
    space: DiscreteSpace[T]

    def move_to(self, cell: T) -> None:
        ...

    def move_relative(self, directions: tuple[int, ...], distance: int = 1):
        ...

The usage of Protocol here allows static duck typing (ignoring the generic T for the moment). That means the move method of Grid2DMovingAgent requires an instance that satisfies the needs of DiscreteSpaceAgent. It could be a CellAgent, but it could also be an instance of any other class, as long as it includes the cell and space attributes and the move_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 types CellAgent. But most of the time the parameters don't actually have to be a subtype of CellAgent, they just need to have a cell attribute. This can be expressed with Protocols.


So, lets discuss these design principles and if we should adopt them.

@quaquel
Copy link
Member

quaquel commented Sep 12, 2024

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.

@EwoutH
Copy link
Member

EwoutH commented Sep 12, 2024

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.

@Corvince
Copy link
Contributor Author

Ah, yes that was the discussion, thanks for digging that up. And yes, this was basically your idea, so some credit goes to you!

@EwoutH
Copy link
Member

EwoutH commented Sep 12, 2024

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.

Comment on lines 64 to 83
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}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Great use of pattern matching!
  2. Should we also allow lower case? If so, we can do direction = direction.lower() and then match against the lower case strings for example.
  3. This now allows "queen" (chess) moves. Do we also want to allow knights and maybe even more? (south-south west, etc.)
  4. Do we want to allow a certain amount of degrees? 0 is up, 90 is east, 135 is south east, etc.
  5. 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Thanks!
  2. Yes, they should also work with lowercase directions I think.
  3. 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
  4. 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!
  5. 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

@EwoutH
Copy link
Member

EwoutH commented Sep 13, 2024

class MyAgent(Agent, CellAgent):
    ...

Obviously I'm in favor of this, since I wrote an whole proposal about this. One thing I would do different is the naming: CellAgent suggests it's a complete Agent. Something like 2DMovement, GridMovement, or MovementBehavior (better names welcome) makes it explicit that it's not an Agent, but some functionality you can add to an Agent.

One of my more coherent thoughts on this I wrote down here:

Behavior and Function Overview

In the context of ABM, we distinguish between behaviors and functions based on their operational characteristics.

  • Function: A function operates in a stateless manner, meaning it doesn't rely on the agent or model's state to function. It can take input arguments and return output variables, just like regular Python functions.
  • Behavior: A behavior operates within the context of an agent's, another agent's, and/or the model's state. It may take this state as input and/or modify it. Behaviors are implemented as classes with a single method bearing the same name as the class/behavior. Generally, behaviors do not require an __init__ method. Users can incorporate behaviors into existing Agent or Model classes using Mixin. A behavior always has access to the self (agent or model) object, and can optionally take a agent, agent_list, or model argument.

(note that agent_list is an artifact from before we had the AgentSet)

So, in this case you're already almost fully adhering to my take on this: A single class Grid2DMovingAgent with a single function move, with the only difference being that I suggested them to carry the same name.

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, InView will generated a set of Agents that are in view of the Agent, and can either:

  1. Return that set (a function in my framework)
  2. Update an Agent state (a behavior)

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 InView could take Agent states and return the set.

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!

@quaquel
Copy link
Member

quaquel commented Sep 13, 2024

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:

A mix-in is a class that defines only a small set of additional methods for its child classes to provide. Mix-in classes don’t define their own instance attributes nor require their init constructor to be called.

Brett, Slatkin. Effective Python: 90 Specific Ways to Write Better Python (Effective Software Development Series) (p. 165). Pearson Education. Kindle Edition.

From this perspective, using multiple inheritance for movement can become tricky. For example, CellAgent relies at the moment on a self.cell attribute. This means we cannot properly handle these movement methods using mixins. One possible solution would be to have a generic position attribute that is part of the Agent class. Movement mixins for either discrete or continous spaces use this attribute. For discrete spaces, position will be a Cell instance, while for continous spaces it is something else (can be as simple as an x,y tuple).

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 __init__ methods unless everythign inside mesa is designed to properly use super because than at least the MRO is well defined and clear.

@EwoutH
Copy link
Member

EwoutH commented Sep 13, 2024

For example, CellAgent relies at the moment on a self.cell attribute.

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 cell attribute which represents the current cell the Agent is in."

But yeah, to do this properly it requires both practices, documentation and continuous checking. It's non-trivial.

@Corvince
Copy link
Contributor Author

From this perspective, using multiple inheritance for movement can become tricky. For example, CellAgent relies at the moment on a self.cell attribute. This means we cannot properly handle these movement methods using mixins.

This is where the Protocol comes into play. The Grid2DMovingAgent defines the move function with a required self type of DiscreteSpaceAgent. So its not a pure mixin, but a restricted mixin (theres probably a different name for that)

@EwoutH
Copy link
Member

EwoutH commented Sep 13, 2024

Protocol seems like a very interesting construct to fill this missing piece of the puzzle (I should have read the PR description better). I'm going to read up on that!

@Corvince
Copy link
Contributor Author

Especially naming of states is crucial to get this working properly

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 Grid2DMovingAgent, which defines a "move" function. If we would rename that to for example MoveGrid2D/move_grid_2d I think thats an awkward name for a simple move. And its not like it could clash with a network_move. Additionally If I swap my model space I would have to change all my movement functions, whereas move could potentially adhere to a single abstract function

@EwoutH
Copy link
Member

EwoutH commented Sep 13, 2024

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.

Good point, I think I agree.

For movement specifically, you would want to check against which space/grid you're working. Agents have a model property, and you could require a model.space property for certain behaviors. Then you can typecheck against that space if it's compatible. Some behaviors might be compatible with a high-level class (like all DiscreteSpace (sub)classes and some other might require a very specific space class (like OrthogonalMooreGrid).

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 @property).

@quaquel
Copy link
Member

quaquel commented Sep 13, 2024

This is where the Protocol comes into play. The Grid2DMovingAgent defines the move function with a required self type of DiscreteSpaceAgent. So its not a pure mixin, but a restricted mixin (theres probably a different name for that)

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 DiscreteSpaces.

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.

@Corvince
Copy link
Contributor Author

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 Protocol I really only see an absolute minimum of

  1. A cell attribute that describes which cell is currently occupied
  2. A move_to method that implements moving from one cell to another cell
  3. A move_along_connection method that moves the Agent along one of the connections of its occupied cell.
    I think this should be sufficient for almost all movements and generally applicable for all discrete spaces. (of course all names are up for discussion). For me that would be the optimal base on which to build upon more specialised behaviour. That means, its a good Protocol. I think we should of course also have a concrete implementation for this, but not subclassing this from Agent also allows for easier testing: CellAgents don't need to have a model attached.

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 [2, 3] you would have {x: "2", y: 3}. This would give the discrete space even more explicit flexibility (especially for n dimensions)

@quaquel
Copy link
Member

quaquel commented Sep 13, 2024

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, move_along_connection can probably be implemented very generically.

I am not sure I fully understand the idea of named coordinates. Would this interact with move_along_connection. So you could say in a 3d grid `move_along_connections(x=3, y=1, z=0), which would mean go one step in the y-direction and 3 steps in the x-direction. For orthogonal grids, this would work. For hexgrids, probably as well, although it requires documenting well how the indexing works. For example if you are on an even row currently, and you want to go up 1 in the y-direction, which cell would this be? This might be easier to explain with a diagram or so.

I am less sure about e.g., a network, which currently just uses single numbers.

@Corvince
Copy link
Contributor Author

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

@EwoutH
Copy link
Member

EwoutH commented Sep 17, 2024

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 Delegation

Instead 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., MovementBehavior) that are instantiated within an agent and manage their own state.

Advantages:

  • Encapsulation: Movement logic is encapsulated within its own class, reducing coupling between agents and movement methods.
  • Flexibility: Agents can dynamically change behaviors at runtime by swapping out behavior instances.
  • Simpler Inheritance: Agents only inherit from the base Agent class, avoiding multiple inheritance complexities.

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:

  • Separation of Concerns: Agents delegate movement responsibilities to the behavior class.
  • Reusability: Movement behaviors can be reused across different agent types.
  • Testing: Movement behaviors can be tested independently from agents.

2. Utilize the Strategy Design Pattern

The 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:

  • Behavioral Variability: Agents can choose from different movement strategies at runtime.
  • Open/Closed Principle: New movement behaviors can be added without modifying existing agent code.

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:

  • Interchangeability: Agents can easily switch movement strategies.
  • Extensibility: Supports adding new movement behaviors without affecting existing code.

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:

  • Enforcement: ABCs can enforce the implementation of required methods.
  • Clarity: Provides a clear contract for what methods and properties an agent must have.

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:

  • Consistency: All agents that are movable adhere to the same interface.
  • Type Checking: Enables both runtime and static checks for interface adherence.

4. Leverage Dependency Injection for Space Awareness

Instead 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:

  • Decoupling: Agents do not need to know about the global space; they operate on injected dependencies.
  • Testability: Easier to mock or replace dependencies during testing.

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 Movement

Shift 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:

  • Centralized Control: Movement rules and constraints can be enforced in one place.
  • Separation of Concerns: Agents focus on deciding to move, not on how movement is executed.

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:

  • Scalability: Easier to manage complex movement rules and interactions.
  • Extensibility: New movement types and rules can be added in the controller.

6. Address Attribute Requirements Through Interfaces

To handle attributes like cell or position required by movement behaviors, define interfaces or use composition to ensure agents have the necessary state without imposing inheritance.

Advantages:

  • Flexibility: Agents can satisfy interface requirements in various ways.
  • Clarity: Movement behaviors document their required attributes through interfaces.

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.

@quaquel
Copy link
Member

quaquel commented Sep 19, 2024

@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 self.movement.move_to or agent.movement.move_to, rather than just self.move_to or agent.move_to. Ideally, I would like to see movement to be part of the interface/protocol of the agent.

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 self.model.grid.move seems particularly verbose to me and conceptually wrong.

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: Agent, ContinousSpaceAgent, DiscreteSpaceAgent that users can start from. I realize that this makes the API a bit more elaborate because a user has to know which base Agent class to start from, but this is relatively easy to explain and document. It also makes it easy to have consistent typing behavior which will help users while developing their own models.

Alternatively, I would also be fine with Agent, ContinuousMovement, and DiscreteMovement and using multiple inheritance: MyAgent(Agent, ContinousMovement) or MyAgent(Agent, DiscreteMovement). Again, it is easy to explain and document. However, multiple inheritance is potentially more fragile in the case of more complex agent hierarchies and larger models. It will also require a bit more work to make the existing Agent class play well with the use of Python super (see here for a short discussion on the problem).

@Corvince
Copy link
Contributor Author

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 DiscreteSpaceAgent is that this is too a bit verbose and specific - after all I think this will be the most used Agent and its a bit hard to explain why its not the default and also why a "normal" Agent shouldn't be allowed to move around in a space.

Also we have to consider how we want to add more behaviours to agents. I think mixin classes (i.e. without __init__) makes the most sense there. But then we again have to explain a bit of multiple inheritance, just without the associated problems of the MRO. But if the hassle is mostly "just" about Agent, DiscreteAgent and ContinuousAgent I think we should just solve that once and be happy.

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

@quaquel
Copy link
Member

quaquel commented Sep 19, 2024

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 MoveableAgent is very clear. Having separate protocols for moving in DiscreteSpaces, Grids, and ContinousSpaces would complement this nicely and will make development in an IDE with typing support easy.

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 __init__). At the moment, CellAgent relies on Cell. I guess we could generalize this to a position attribute (which is implicitly done in Agent atm via Agent.pos. As long as position is either a Cell or an equivalent Class for a continuous space, we might be well positioned.

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.

@EwoutH
Copy link
Member

EwoutH commented Sep 19, 2024

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

That's exactly what I envisioned in #2292 (comment), so I think we fully align on this!

I think this will be the most used Agent and its a bit hard to explain why its not the default and also why a "normal" Agent shouldn't be allowed to move around in a space.

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.


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 self.model.grid.move seems particularly verbose to me and conceptually wrong.

I find this a difficult one. Because:

  1. Movement is imitated from the Agent, which wants to move.
  2. Which movement is allowed, which isn't, what if a cell if full, what if it is or isn't a neighbour, etc. etc. are all Space dependent.

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

  1. Like we reserved Agent.model, we reserve Agent.space. The API Agents.space.move seems quite acceptable. This also allows Agents to be on multiple spaces (space2, etc.), and request moves on different spaces. The PropertyLayer for example was designed to support a "layered" model of the environment/spaces, where you can stack multiple layers/spaces onto each other for GIS-like lookups. This is basically option 5, but refined.
  2. The other logical option is doing some kind of injection of a movement class/functions, which support some (range of) grids. In Mesa, I think we can indeed abstract that to a Moveable class that supports all spaces in Mesa. That class should have method to detect which space is used, and than perform the request based on that space.

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.

@Corvince
Copy link
Contributor Author

I find this a difficult one. Because:

  1. Movement is imitated from the Agent, which wants to move.
  2. Which movement is allowed, which isn't, what if a cell if full, what if it is or isn't a neighbour, etc. etc. are all Space dependent.

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.

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 self.model.space makes more sense in my opinion.

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.

Exactly, this gets too bloated quickly I believe. But lets not overcomplicate things. I think we only have 3 states

  1. Agent has no space
  2. Agent has a discrete space (grid, network, voronoi, (more?)
  3. Agent has a continuous space

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 .cell and .position as separate attributes)

@quaquel
Copy link
Member

quaquel commented Sep 19, 2024

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 .cell and .position as separate attributes)

I am still wondering whether it would be possible to use a common interface on a position in a DiscreteSpace (i.e., a Cell) and a position (i.e., some coordinate) in a ContinuousSpace. If the ContinuousSpace is implemented internally using a KD-Tree, this might be possible. You could then query your position for nearest neighbors or neighbors within a radius (both are common operations in a KD-Tree). So self.position.get_neigbors(radius=5) or self.position.nearest_neigbor. This is very close to what we already have with Cell. Cell then moves beyond this by distinguishing neighboring positions (i.e., cells) and neighboring agents. Meanwhile, a position in a ContinuousSpace might only have neighboring agents.

Next, you might enrich this minimum form of movement further, but most of that would be built on top of this common interface.

@wang-boyu
Copy link
Member

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 traits when we talked about mesa & signals (#2281 (comment)))?

For example, neighborhood search in contiuous spaces might be faster if the space internally uses a KD -tree.

This has been explored in #1619. Mesa-Geo uses rtree and lazy-evaluation for spatial indexing (projectmesa/mesa-geo#179).

@quaquel
Copy link
Member

quaquel commented Sep 21, 2024

Is this similar to traits in Rust

Yes! That is exactly what we are talking about, but then trying to make it work within Python's OO multiple inheritance structure.

@Corvince
Copy link
Contributor Author

Corvince commented Oct 1, 2024

From #2333 (comment)

Now that this Agent is moving, this name might get confusing with an Agent that sits in a fixed place, and the whole grid is covered with such (like the patches in NetLogo). For now fine, but maybe something we should keep in mind as the cell space develops.

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

@quaquel
Copy link
Member

quaquel commented Oct 2, 2024

I have added tests for the new stuff, which is now fully covered. I also changed a few minor things. Grid2DMovingAgent now extends CellAgent. I am not sure this is correct. Should Grid2DMovingAgent not be split into Grid2DMovevement(BasicMovement) and Grid2DMovingAgent(CellAgent, Grid2DMovevement) ? So separating the movement method from the Agent class?

I also added a custom remove to Patch. There is a fixme comment that is worth reading. Basically, Patch.remove() removes the patch from the cell but leaves self.cell to the original value instead of setting it to None. This means that the Patch can never be reassigned to any Cell. Otherwise, you could use remove to indirectly move patches around:

# 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

@Corvince
Copy link
Contributor Author

Corvince commented Oct 2, 2024

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?
And I am honestly not sure if that's needed. It's good that Patch doesn't have any movement methods and prevents you from accidently moving it. But if someone goes the extra mile of removing and adding it somewhere else they probably have some reason for that? So maybe that would be an ok workaround. Or maybe not

@quaquel
Copy link
Member

quaquel commented Oct 2, 2024

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.

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.

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?
And I am honestly not sure if that's needed. It's good that Patch doesn't have any movement methods and prevents you from accidently moving it. But if someone goes the extra mile of removing and adding it somewhere else they probably have some reason for that? So maybe that would be an ok workaround. Or maybe not

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 Patch.remove. On the other hand, otherwise FixedCell.__set__ becomes quite a bit more complicated because you would need to check if cell is None and allow that as a valid value.

@quaquel quaquel added experimental Release notes label enhancement Release notes label labels Oct 3, 2024
@Corvince
Copy link
Contributor Author

Corvince commented Oct 3, 2024

got it now, thanks!

Copy link
Member

@EwoutH EwoutH left a 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.

mesa/experimental/cell_space/cell_agent.py Outdated Show resolved Hide resolved
mesa/experimental/cell_space/cell_agent.py Show resolved Hide resolved
@EwoutH
Copy link
Member

EwoutH commented Oct 3, 2024

Let me guess, the DIRECTION idea came from an llm? At least thats what was offered to me yesterday.

Conceptually no, implementation yes ;)

Means we're asking the same questions, great minds think alike? 😇

Grid2DMovingAgent now extends CellAgent. I am not sure this is correct. Should Grid2DMovingAgent not be split into Grid2DMovevement(BasicMovement) and Grid2DMovingAgent(CellAgent, Grid2DMovevement) ? So separating the movement method from the Agent class?

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.

This means that the Patch can never be reassigned to any Cell

Smart!

Co-authored-by: Ewout ter Hoeven <[email protected]>
@Corvince
Copy link
Contributor Author

Corvince commented Oct 4, 2024

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?

@EwoutH
Copy link
Member

EwoutH commented Oct 4, 2024

No worries, we got you. Updated the tests and also directly used the FixedAgent in a benchmark and example model.

I'm good to merge, @quaquel?

@quaquel quaquel added trigger-benchmarks Special label that triggers the benchmarking CI and removed trigger-benchmarks Special label that triggers the benchmarking CI labels Oct 4, 2024
Copy link

github-actions bot commented Oct 4, 2024

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 -0.5% [-1.3%, +0.2%] 🔵 -1.4% [-1.5%, -1.3%]
BoltzmannWealth large 🔵 -0.6% [-1.1%, -0.1%] 🔵 -1.6% [-2.8%, -0.6%]
Schelling small 🔵 +0.6% [+0.3%, +0.8%] 🔵 +0.0% [-0.2%, +0.3%]
Schelling large 🔵 +0.4% [-0.2%, +0.9%] 🔵 -0.1% [-1.9%, +1.7%]
WolfSheep small 🔵 +0.4% [+0.2%, +0.7%] 🔵 +0.6% [+0.4%, +0.8%]
WolfSheep large 🔵 +1.2% [-0.2%, +2.7%] 🔵 +2.1% [+0.6%, +3.7%]
BoidFlockers small 🔵 -0.7% [-1.0%, -0.3%] 🔵 -0.2% [-0.7%, +0.4%]
BoidFlockers large 🔵 -0.8% [-1.2%, -0.4%] 🔵 -0.5% [-1.0%, +0.1%]

@quaquel quaquel merged commit a7dc9b2 into main Oct 4, 2024
11 checks passed
@quaquel quaquel deleted the cell-space-agents branch October 4, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Release notes label experimental Release notes label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants