Skip to content

Conversation

@quaquel
Copy link
Member

@quaquel quaquel commented Nov 3, 2025

A draft and WIP PR exploring ideas discussed here

This PR only turns HasCell and FixedCell into descriptors and changes nothing else.

@quaquel quaquel requested a review from EwoutH November 3, 2025 18:25
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔴 +12.1% [+11.0%, +13.0%] 🔴 +3.8% [+3.6%, +3.9%]
BoltzmannWealth large 🔴 +7.8% [+6.6%, +8.7%] 🔴 +11.7% [+9.6%, +13.8%]
Schelling small 🔴 +4.9% [+4.7%, +5.2%] 🔵 +2.6% [+2.4%, +2.8%]
Schelling large 🔴 +3.9% [+3.5%, +4.3%] 🔴 +4.2% [+3.2%, +5.3%]
WolfSheep small 🔴 +10.5% [+10.3%, +10.7%] 🔴 +5.0% [+4.9%, +5.2%]
WolfSheep large 🔴 +11.0% [+10.4%, +11.7%] 🔴 +9.3% [+8.8%, +9.8%]
BoidFlockers small 🔵 +0.1% [-0.2%, +0.4%] 🔵 -0.7% [-0.9%, -0.6%]
BoidFlockers large 🔵 +0.1% [-0.5%, +0.7%] 🔵 -0.5% [-0.8%, -0.3%]

@EwoutH
Copy link
Member

EwoutH commented Nov 3, 2025

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

@quaquel
Copy link
Member Author

quaquel commented Nov 3, 2025

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

@quaquel
Copy link
Member Author

quaquel commented Nov 5, 2025

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

@EwoutH
Copy link
Member

EwoutH commented Nov 7, 2025

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.

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

Successfully merging this pull request may close these issues.

2 participants