Skip to content

Enforce read-only safety for 'empty' layer#3080

Closed
codebreaker32 wants to merge 6 commits intomesa:mainfrom
codebreaker32:feat/create-readonlydescriptor
Closed

Enforce read-only safety for 'empty' layer#3080
codebreaker32 wants to merge 6 commits intomesa:mainfrom
codebreaker32:feat/create-readonlydescriptor

Conversation

@codebreaker32
Copy link
Copy Markdown
Collaborator

@codebreaker32 codebreaker32 commented Jan 6, 2026

Summary

This PR implements a robust mechanism for Read-Only Property Layers, primarily to secure the cell.empty state. It utilizes a Polymorphism pattern to ensure that the PropertyLayer remains perfectly synchronized with Cell agent lists, while maintaining strict decoupling between the Cell class and the Grid.

Motive

See #3072

Implementation

  1. mesa/discrete_space/property_layer.py:
  • Added ReadOnlyPropertyDescriptor: A specific descriptor class that raises AttributeError on __set__.
  • Updated HasPropertyLayers: Added a closure(_make_setter) that captures the specific layer instance, allowing the Cell to write to the NumPy array without holding a reference to it.
  1. mesa/discrete_space/cell.py:
  • Replaced direct property assignment (self.empty = val) with a protected method call (self._set_empty(val)).
  • Added a default implementation of _set_empty that updates a local variable, ensuring the Cell works correctly in non-grid contexts (e.g., Network).
  1. mesa/discrete_space/grid.py:
  • Initialized the default empty layer with read_only=True.

Attempting to manually modify the state now raises an AttributeError

cell = grid[(0,0)]
try:
    cell.empty = False
except AttributeError as e:
    print(e)
# Output: Property 'empty' is read-only....

Additional Notes

  • Verification: I have run tests locally to verify that the injection works correctly and that Network instances (which lack property layers) fallback gracefully to the default local storage.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 6, 2026

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 +0.4% [-1.6%, +2.3%] 🟢 -4.4% [-4.6%, -4.2%]
BoltzmannWealth large 🔵 -1.0% [-2.2%, +0.1%] 🔵 +4.5% [-2.1%, +11.9%]
Schelling small 🔵 -0.3% [-1.1%, +0.6%] 🔵 +1.4% [+0.8%, +1.9%]
Schelling large 🔵 -0.1% [-2.2%, +2.6%] 🔵 -4.2% [-7.1%, -1.0%]
WolfSheep small 🟢 -3.4% [-3.7%, -3.1%] 🔵 -2.3% [-2.6%, -1.9%]
WolfSheep large 🔵 -2.3% [-5.2%, +1.6%] 🔵 -2.3% [-4.3%, -0.3%]
BoidFlockers small 🔵 -1.4% [-1.9%, -0.9%] 🔵 -0.1% [-0.5%, +0.3%]
BoidFlockers large 🔵 -2.0% [-2.7%, -1.2%] 🔵 -1.1% [-1.7%, -0.5%]

@codebreaker32
Copy link
Copy Markdown
Collaborator Author

Please review @quaquel

@codebreaker32 codebreaker32 changed the title Enforce read-only safety for 'empty' layer while maintaining Grid-Cell decoupling Enforce read-only safety for 'empty' layer Jan 7, 2026
@quaquel quaquel added the enhancement Release notes label label Jan 7, 2026
@quaquel
Copy link
Copy Markdown
Member

quaquel commented Jan 7, 2026

Thanks for this PR. I hope to take a closer look somewhere this week. However, I also have a few ideas of my own that I want to test, and since is_empty is performance-critical in my profiling of all examples, I want to rigorously profile any solution before accepting it.

@quaquel
Copy link
Copy Markdown
Member

quaquel commented Jan 7, 2026

As indicated, empty is performance critical. So, I did a quick comparison of an attribute, a property, and a descriptor.

Setting and getting a normal attribute is evidently the fastest. Setting a property is an order of magnitude slower. Setting via a descriptor is double the time of a property. Getting a property is about a factor of 5 more expensive, while via descriptor is an order of magnitude slower compared to a normal attribute.

So what does this mean? To make cell.property work, we cannot avoid using descriptors. However, I am wondering whether we need to implement empty as a property layer. Currently, this property layer is used in some of the mask functions of HasPropertyLayer, but it is the only internal function that utilizes it.

So, what about turning empty simply into a numpy array instead of using the property layer indirection? This is, for now, a discussion question, so please refrain from modifying this PR just yet.

@codebreaker32
Copy link
Copy Markdown
Collaborator Author

codebreaker32 commented Jan 7, 2026

I agree that descriptors introduce overhead compared to raw attributes. However, I believe replacing PropertyLayer with a raw NumPy array will not solve the bottleneck you identified, but it will fracture the API consistency.

The "Bottleneck" is the Descriptor, not the Layer Object

To make cell.property work, we cannot avoid using descriptors.

Agree. whether cell.empty reads from PropertyLayer.data or a raw numpy_array, we are forced to use the descriptor (the overhead of __get__ and __set__ calls).

  • Current Path: descriptor -> layer.data -> numpy
  • Raw Array Path: descriptor -> numpy

The difference is a single Python attribute lookup (.data). This is negligible compared to the "Order of Magnitude" cost of the descriptor call itself. Switching to a raw array won't make cell.empty meaningfully faster.

The Cost of Dropping PropertyLayer
If we turn empty into a raw array:

  • Breaks select_cells: Operations like grid.select_cells(only_empty=True) rely on grid.properties['empty'] existing. Special-casing this creates hassle.

but it is the only internal function that utilizes it.

A raw array would also need manual state management which HasPropertyLayers does for us
However If we wish to remove the PropertyLayer, I think the performance gain should be worth the hassle

@codebreaker32
Copy link
Copy Markdown
Collaborator Author

codebreaker32 commented Jan 7, 2026

@quaquel
I honestly believe that using propertylayer(or numpy array) has a lot of scope in Mesa
For eg. I noticed(now)
In mesa/discrete_space/grid.py, the method looks like this:

def select_random_empty_cell(self) -> T:
    # 1. Fast Heuristic 
    if self._try_random:
        for _ in range(50):
            # ... tries random cells ...

    # 2. Slow Fallback (Iterate EVERYTHING)
    # This calls DiscreteSpace.select_random_empty_cell
    # which runs: return self.random.choice(list(self.empties))
    # 'self.empties' runs: return self.all_cells.select(lambda cell: cell.is_empty)
    return super().select_random_empty_cell()

When the grid is full (e.g., >90% density), the heuristic fails. The fallback then forces Python to check cell.is_empty on every single cell to build a list, just to pick one. On a 100x100 grid, that's 10,000 checks.

We can improve it with numpy

def select_random_empty_cell(self) -> T:
    if self._try_random:
        for _ in range(50):
            cell = self.all_cells.select_random_cell()
            if cell.is_empty:
                return cell

    # 2. Vectorized Fallback
    # Instead of iterating objects, ask NumPy for the coordinates of all empty cells.
    # self.select_cells is provided by HasPropertyLayers
    empty_coords = self.select_cells(only_empty=True)  #

    # 3. Pick one coordinate and retrieve the cell
    random_coord = self.random.choice(empty_coords)
    return self._cells[random_coord]

I believe PropertyLayer is not just wrapper overhead; it enables optimizations that are impossible with just raw loops.

@quaquel
Copy link
Copy Markdown
Member

quaquel commented Jan 7, 2026

My longer-term thinking is what you call the raw array path. That is, getting rid of the PropertyLayer class entirely. This removes one layer of indirection, which should result in performance benefits.

I also agree that there is a lot more that could be done with space.empty than is currently done. If you check the git blame, you can see that it was added as part of the PR that introduced support for property layers. None of the existing prior functionality was rewritten to utilize it. And yes, I have been looking at the random cell selection as well and see obvious benefits to using space.empty here.

However, space.empty is also a bit different from other property layers, most notably in its read-only character. This raises the question of whether, in the long term, we should implement it in exactly the same way. For example, we might use custom properties instead of a descriptor. This halves the call overhead.

Since raising the idea of read-only property layers, I have also been wondering about other examples where one might need the protection of read-only access. For empty, it is clearly necessary due to its internal importance. But I cannot really envision other property layer use cases where you want to have this read-only protection.

@codebreaker32
Copy link
Copy Markdown
Collaborator Author

Thanks for sharing! I completely agree that flattening the PropertyLayer indirection seems like the logical end state for maximum performance. It is also validating to hear that we are aligned on the potential for empty to drive better vectorization logic (like the random selection we discussed).

I am very curious about your point on "custom properties" halving the overhead compared to descriptors. By this, do you mean generating specific @property methods on the Cell class (dynamically or statically) to leverage the optimized C-implementation of python properties, rather than routing through a generic Python-based __get__ descriptor class? That sounds like a fascinating optimization path to explore.

I am curious to see how it turns out and I will surely follow the discussions :)

@quaquel
Copy link
Copy Markdown
Member

quaquel commented Jan 7, 2026

I am very curious about your point on "custom properties" halving the overhead compared to descriptors. By this, do you mean generating specific @Property methods on the Cell class (dynamically or statically) to leverage the optimized C-implementation of python properties, rather than routing through a generic Python-based get descriptor class? That sounds like a fascinating optimization path to explore.

I ran tests with the following code. So I am comparing @property with a Descriptor class. Of course, on our use case, performance might be different, but it's still clear that there is just more overhead with a Descriptor class. I am not sure why this happens. According to the Descriptor Guide, data descriptors are evaluated first in the method resolution order. It's possible that getattr is the primary cause of the slowdown, but I would need to investigate further to gain a comprehensive understanding.

class MyClass_A:
    def __init__(self):
        self.a =1 

class MyClass_B:

    @property
    def a(self):
        return self._a

    @a.setter
    def a(self, value):
        self._a = value


    def __init__(self):
        self._a =1 

class Property:
    def __set_name__(self, owner, name):
        self.private_name = f"_{name}"

    def __set__(self, obj, value):
        setattr(obj, self.private_name, value)

    def __get__(self, obj, objtype=None):
        return getattr(obj, self.private_name)

class MyClass_C:
    a = Property()
    
    def __init__(self):
        self.a =1     

@codebreaker32
Copy link
Copy Markdown
Collaborator Author

@quaquel Thanks for sharing your long-term vision. After careful reading(descriptor vs @property) and researching the motives for removing PropertyLayer, I came across some conclusions:-

It confirms that the overhead comes from the Python-level function calls (__get__) and dictionary lookups (getattr) inherent to custom descriptors. Native @property slots avoid most of this by handling access at the C level.
To achieve this performance dynamically, we might look to implement a property factory pattern.

Instead of attaching a PropertyDescriptor instance to the cell class, we may define a helper that creates a closure (capturing the data array directly) and returns a native property object. We can then attach these C-optimized properties to the dynamically generated cell_klass at runtime.
This avoids the __get__ overhead entirely and should structurally match the performance of your MyClass_B example while keeping the API flexible with minimum overheads.

It also led me to thought: If we go down this path, we don't need the PropertyLayer class at all

If the Cell communicates directly with the array via the property closure, PropertyLayer effectively becomes useless(an object that exists only to hold the NumPy array without adding state value because all its methods are generic numpy methods).
How it might look like after removal;-

  • Grid.properties becomes a simple Dict[str, np.ndarray].
  • The Cell class uses dynamic native properties (closures) pointing to those arrays.
  • Vectorized operations (select_cells, etc.) move to the Grid class or a utility module, operating on the numpy arrays.

I plan to spend more time researching this architecture and will try to come up with a faster solution with flattened hierarchy. If you feel I’m heading in the wrong direction or there’s a better approach, I’d really appreciate your guidance :)

@quaquel
Copy link
Copy Markdown
Member

quaquel commented Jan 8, 2026

Feel free to submit any pathfinding style draft PR so I can review it as well. For this, don't worry too much about backward compatibility. Its highly likely that we'll open a Mesa 4 branch in the coming days to weeks anyway.

@quaquel
Copy link
Copy Markdown
Member

quaquel commented Jan 11, 2026

@codebreaker32, in case you are curious about your idea of a property factory pattern, please check #3113, where I am doing some pathfinding and have included this.

@quaquel
Copy link
Copy Markdown
Member

quaquel commented Jan 15, 2026

Thanks again for this PR! Some of the ideas have already been implemented. However, the current plan is to first make property layers work for all discrete spaces by explicitly separating their x,y position from their logical index. Next, we can revisit this PR. All this will be part of the mesa 4 development. So I am closing this PR but will revisit the idea hopefully soon.

@quaquel quaquel closed this Jan 25, 2026
@codebreaker32 codebreaker32 deleted the feat/create-readonlydescriptor branch February 17, 2026 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Release notes label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants