Performance benchmarks:

If I'm correct, by shifting from inheritance-based mixins to descriptors, this enables to define agents like this, right?

# Before (Inheritance)
class CellAgent(Agent, HasCell, BasicMovement):
    # HasCell provides cell property via inheritance
    pass

# After (Descriptors)
class CellAgent(Agent, BasicMovement):
    cell = HasCell()  # Descriptor manages cell attribute

Would this enable composition-based approaches (as discussed)?

# Future possibility: Multiple locations via descriptors
class HybridAgent(Agent):
    physical_cell = HasCell()
    logical_cell = HasCell()
    
    def step(self):
        # Agent can exist in multiple discrete spaces
        neighbors_physical = self.physical_cell.get_neighborhood()
        neighbors_logical = self.logical_cell.get_neighborhood()

Yes, the code example you have given will work fine with the new descriptor.

I had a look at creating a HasPosition descriptor for the new experimental ContinuousSpace. The problem here is the following. We currently have the following properties:

    @property
    def position(self) -> np.ndarray:
        """Position of the agent."""
        return self.space.agent_positions[self.space._agent_to_index[self]]

    def position(self, value: np.ndarray) -> None:
        if not self.space.in_bounds(value):
            if self.space.torus:
                value = self.space.torus_correct(value)
            else:
                raise ValueError(f"point {value} is outside the bounds of the space")

        self.space.agent_positions[self.space._agent_to_index[self]] = value

Note how these properties rely on self.space. Because of this reliance of knowing the space, we cannot straightforwardly create a HasLocation descriptor. The following is a straightforward translation of the properties into a descriptor

class HasPosition:

    def __get__(self.obj: Agent, type=None)
        """Position of the agent."""
        return obj.space.agent_positions[obj.space._agent_to_index[self]]

    def __set__(self, obj: Agent, value: np.ndarray) 
        if not obj.space.in_bounds(value):
            if obj.space.torus:
                value = obj.space.torus_correct(value)
            else:
                raise ValueError(f"point {value} is outside the bounds of the space")

        obj.space.agent_positions[self.space._agent_to_index[self]] = value
 

Note how we are using obj.space repeatedly. This means that we have now hardcoded the name of the attribute to which the space should be assigned inside the agent. This also means that this descriptor is not compatible with multiple spaces. So can we do better? One solution is to do

class HasPosition:
    def __init__(self, space_attribute_name:stre):
        self.space_attribute_name = space_attribute_name

    def __get__(self.obj: Agent, type=None)
        """Position of the agent."""
        space = getattr(obj, self.space_attribute_name)
        return space.agent_positions[space._agent_to_index[self]]

    def __set__(self, obj: Agent, value: np.ndarray) 
    	space = getattr(obj, self.space_attribute_name)
        if not space.in_bounds(value):
            if space.torus:
                value = space.torus_correct(value)
            else:
                raise ValueError(f"point {value} is outside the bounds of the space")

        space.agent_positions[self.space._agent_to_index[self]] = value
  
# we use this accordingly

def MyAgent(Agent)
    my_location = HasPosition("my_space")

    def __init__(self, model):
        super().__init__(model)
        self.my_space = model.space

We now pass the name of the space attribute as a string to HasPosition, and we must ensure that we assign the correct space to this attribute. This approach will work with multiple spaces:

def MyAgent(Agent)
    my_location = HasPosition("my_space")
    my_other_location = HasPosition("my_other_space")

    def __init__(self, model):
        super().__init__(model)
        self.my_space = model.space_1
        self.my_other_space = model.space_2

The drawback of this is the fact that it's up to the user to ensure that the string and the attribute name in the init match. Another drawback is that it's not easy to do neighborhood stuff in this way. All of this will have to be defined at the agent level. So an alternative solution is to introduce a new Location class, which would be used like this

def MyAgent(Agent)
    my_location = HasPosition()

    def __init__(self, mode, position:np.ndarrayl):
        super().__init__(model)
        self.my_location = Location(position, space=model.space)

I haven't fully fleshed out the details of this new Location class, but in my view, the following things should all work

self.location  += [0.1, 0.1]
self.location  *=  2
self.location  = self.location + time_delta * velocity
self.location.get_neighbors_in_radius(5)
self.location.get_nearest_neighbors()

So my 2 questions:

1. What do people think of the use of a string to specify the space attribute?
2. What do people think about a new Location class

I think we're converging towards the same ideas. I don't like the string to specify the space attribute, but I think it's necessary evil. You have to link them somehow.

It would be ideal though, if one location could be linked to multiple spaces.

A magic hack we could (I'm not saying should!) do, is defaulting to HasPosition(space="space"). This way using .space in the model magically works, while it's still flexible to modify. However it does violate explicit over implicit.

I don't understand the the Location class fully, but if it can work as in the API usage snippet you shared, that would be great.

1. I think there are two separate issues here. The first issue is to make it possible to have a model with multiple spaces and agents having separate locations in each of these spaces. The second is having a single location that is somehow valid in multiple "spaces". This draft PR primarily focuses on addressing the first issue, rather than the second.
2. Yes, it will be trivial to implement a Location in line with the sketched API. The advantage is that in that case, we don't need the string.
3. Having a location that is somehow valid in multiple "spaces" is, at face value, similar to what we have already done with property layers for grids. The question is whether we can generalize the idea of property layers and create "derived spaces". That is, we have a primary space in which the location is defined, and we have other derived spaces that use the location from the main space but might have their own custom distance metric that determines neighborhood relations between locations.

3. The question is whether we can generalize the idea of property layers and create "derived spaces".

Interesting idea of having a "primary" space and derived ones. The primary automatically feels like it should be a space with resolution equal or higher than all other spaces. The continuous space has infinite resolution, so it seems logical that a grid-based space derives its location from there. You move in continous space, but can check in which cells of discrete spaces you are. Cell centroids and network nodes and edges could also help "guide" you towards/on a specific location.

@wang-boyu have you been following the recent spaces discussions?

@quaquel I think this idea is great! It would be awesome to be able to see an agent form multiple perspectives at any given moment.

My preference would be for the Location class instead of the string. I think it will provide greater flexibility and extensibility in the end.

Thanks for the feedback on the Location idea. I'd like to move this PR forward and keep it atomic with just the change for discrete spaces. I have stared work on the Location class but its a bit trickier that I would have liked so I'll leave that for a future PR. This is now ready for review.

Performance benchmarks:

I will try to review tomorrow.

That code block can indeed be removed (I had already done so locally, but forgot to push the change).

It would be really nice to have this feature. The only thing I'm concerned about is the performance.

If I'm correct:

# Mixin approach
agent.cell  # Direct attribute access

# Descriptor approach  
agent.cell  # Calls __get__ → getattr(obj, "_cell") → exception handling

BoltzmannWealth shows ~+8% runtime increase. Could be worse, but not ideal, especially since not all models will benefit from this (only if using multiple spaces).

Performance benchmarks:

The current implementation uses a property, so its not entirely direct attribute access, but yes using a descriptor adds overhead. So the question here is whether being able to have an agent in multiple discrete spaces out of the box is worth the performance overhead? There is another option: keep the old HasCell and simply add a HasCellDescriptor. This maintains the performance boost for 80% of use cases (single space) while allowing multiple spaces to be specified via the descriptor.

The current HasCell isn’t used by users directly, right? Can we make this fast option private and use it internally, and expose the new slower but more flexible option to users that want to use multiple spaces?

The current HasCell isn’t used by users directly, right? Can we make this fast option private and use it internally, and expose the new slower but more flexible option to users that want to use multiple spaces?

At present, on main, HasCell is already not included in __all__ for mesa.discrete_space, so it is not part of the public API. So, yes, we could just add a new HasCellDescriptor while keeping HasCell and use it only internally.

Then personally I think this is the way to go, even if it means some code duplication.

add a new HasCellDescriptor

For naming, would something like CellLocation make sense? And then in the future we could add ContinuousLocation and NetworkLocation. Or maybe -Position.

@EwoutH , I updated the PR by adding 2 descriptors: FixedCellPosition and CellPosition. These descriptors serve the same purpose as the mixins: HasCell and FixedCell, which are used internally for performance reasons.

Awesome! I think this is a clear API.

Do we want to make _HasCell and _FixedCell private?

Further, since we're in the stable space, some tests might be nice. Especially one with the intended purpose of having multiple position variables in an Agent, and maybe even being present in multiple spaces.

Do we want to make _HasCell and _FixedCell private?

The use of underscores is for private methods and attributes (at least to the best of my knowledge). By not including HasCell and FixedCell in mesa.discrete_space.__init__.__all__, we are already explicitly making them not part of the public API, thus achieving the same effect.

Further, since we're in the stable space, some tests might be nice. Especially one with the intended purpose of having multiple position variables in an Agent, and maybe even being present in multiple spaces.

I agree, will try to fix that later today.