Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Performance benchmarks:
|
Switch the Ruff pre-commit hook from the deprecated `ruff` legacy alias to the canonical `ruff-check` hook ID, aligning the configuration with upstream changes in `ruff-pre-commit` and eliminating the “legacy alias” warning while preserving existing linting and auto-fix behavior. See astral-sh/ruff-pre-commit#124
codebreaker32
left a comment
There was a problem hiding this comment.
I think this will be faster
|
In self.cell_klass = type(
"GridCell",
(self.cell_klass,),
{"_mesa_properties": set()},
)Since this |
I have updated the PR to include this change, as well as added a test for it (and fixed it in a few other places not covered in #3108). |
|
Performance benchmarks:
|
|
I split of the dict/slots stuff into its own stand alone PR. I now understand how it works and its a nice stand alone PR. I also split of the |
This PR removes
__dict__and@cached_propertyfromCell,This is in line with #3108 and reduces the memory footprint of a grid. This works because only@cached_propertyrequiresCell.__dict__. Properties, descriptors, but also@cachedon't needCell.__dict__, because they rely on storage at the class-level, not the instance-level.Motivation for removing
__dict__As indicated by @falloficarus22 in #3108,
It also replaces the
PropertyDescriptorwith a property factory pattern, as suggested by @codebreaker32 in #3080. But, I still need to include thecell.emptyread-only content from #3080.So far, it appears that there is a slight performance loss from removing
@cached_property. This is largely due to the extra method call from the property toget_neighborhood, which is still cached. Shifting fromself.cell.neighborhoodtoself.cell.get_neighborhood()avoids the indirection, which has a small performance benefit. For Mesa 4.0, we may consider removingcell.neighborhood, as we currently have two different ways of obtaining a neighborhood with a radius of 1 and the center excluded.Shifting from a descriptor to a property halves the time to access it. There might be room for further improvement here.
cell.emptyin main works because ofGridadding a property layer. However, as per Enforce read-only safety for 'empty' layer #3080, empty is a bit of a weird case.Cell.emptyis special. The cell itself should be able to write to it, but the user should not be able to. One option is to rename it to_emptyto clearly signal that it is used internally and should be left alone by the user. In which case, we can still use property layers. Another option is to avoid usingself.add_property_layerand instead work around it forempty.Regardless, a benefit of shifting from a descriptor to a property factory is that write-only access becomes trivial. The
property()function takes as keyword arguments a getter, a setter, and a deleter. By omitting the setter, you have automatically created read-only access.