-
Notifications
You must be signed in to change notification settings - Fork 917
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
Add full support for property layers to cell spaces #2512
Conversation
for more information, see https://pre-commit.ci
This comment was marked as outdated.
This comment was marked as outdated.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
This is now working again. It required some Python tricks involving dynamically creating subclasses. |
Performance benchmarks:
|
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Deepcopy and pickle now also work with property layers. It took a bit longer than expected, but I learned a lot about pickle and deepcopy internals. In the end, one last item left: how do we want to access property layers at the grid level? There are at least 3 options
Of these, I am inclined to go with option 1. You can then do |
for more information, see https://pre-commit.ci
For completion, I think technically there's a fourth option, like Pandas does it to access series:
However, 1 feels really great once you know it, and there is precedence for this kind of attribute acces in the Python framework: arr = np.array([[1, 2], [3, 4]])
arr.shape # (2, 2)
df = pd.DataFrame({'a': [1, 2], 'b': [3, 4]})
df.a # Series([1, 2]) There should be some checks on the variable names, at least they should be valid Python identifiers (similar to what we now enforce in the EMA workbench). Maybe we should also check against some other things. Best to see is how NumPy and Pandas do it, they probably already figured this out fully. Maybe packages like nameutils could also help out here. |
pandas does not do a check, but if it is not a valid identifier, the attribute like access is simply not supported. So you can have column names with a space in them and pandas accepts that. I am inclined to just follow that route and document clearly that for attribute access, you need to ensure that the property layer name needs to be a valid python identifier. Note that the package you link to does something interesting but quite different (namely handling family names and capitalization). |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
@quaquel Thanks for continuing working on this. How far do you expect it from being ready to be merged? |
Its basically done, but I like to give it one last look over before merging. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
I updated the PR description based on the final implementation, could you check it? |
Summary
This PR adds full support for PropertyLayers to the experimental cell spaces in Mesa. It introduces a new
property_layers
module and integrates it with the cell space architecture, offering an enhanced, n-dimensional implementation with attribute-like property access for cells.Motive
While Mesa already had PropertyLayers in
mesa.space
, they needed adaptation to work seamlessly with the new cell spaces. This implementation:Implementation
The key implementation changes include:
Created a new
property_layers
module inmesa.experimental.cell_space
containing:HasPropertyLayers
mixin class for adding property layer support to gridsPropertyDescriptor
class enabling attribute-like access to propertiesPropertyLayer
classCore improvements:
cell.grass
instead ofcell.get_property("grass")
)empty
property layer to track cell occupancyArchitectural changes:
DiscreteSpace
toGrid
Usage Examples
Additional Notes
Original description
This is a somewhat opinionated take on how to offer full support for property layers in new-style cell spaces. It draws on ideas from #2431 and #2440. In short, I have taken the property layer code from
mesa.space
, and created a newproperty_layers
module inmesa.experimental.cell_space
. This ensures that the existing code inmesa.space
remains untouched, and I can tune the behavior to align with the design ideas of cell_space.All relevant code for adding property layer behavior to grids is contained in a new
HasPropertyLayer
class. This is primarily for easy development and allowing me to understand better what is going on.I have moved property layer functionality from the generic
DiscreteSpace
toGrid
as discussed in #2431. Because Grid can be n-dimensional, I modified PropertyLayer and all relevant code to work for n-dimensional grids.I enforce that the dimensionality of the property layer and the dimensionality of the grid are the same. This was left implicit before. Now, it raises errors if you try to have a layer with a different coordinate system.
Cells have attribute-like access to their properties. So
cell.grass
orcell.elevation
. Likewise, any modification done at the cell level can just use assignment (i.e.,=
). This is all implemented via aPropertyDescriptor
, which offers a view into the underlying numpy array defined in the property layer.What currently does not work yet is:
Masks (this is a trivial problem, just a bit more tricky because it needs to work for n-dimensional grids).Theempty
property layer is not yet working. I have to take a closer look at this. It involves two separate problems: (1) Where should the layer be added? It cannot be done in the__init__
ofHasPropertyLayers
because when this code is executed, the__init__
ofGrid
has not been completed. So e.g.,dimensions
is not yet defined. (2) It requires updatingCell
to set a boolean to true or false. Since I am relying on adding descriptors dynamically, but only for grids and not for e.g.,Network
, I have to figure out how to make this all work correctly independently of the exact subclass ofDiscreteSpace
to whichCell
belongs. Ideally, the Cell class should be unaware of the presence or absence of anempty
property layerHow the dynamic adding of descriptors interacts with deepcopy and pickle operations. Again, I have no idea here, but it should be fixable.