Skip to content

Replace PropertyDescriptor with properties#3125

Merged
quaquel merged 4 commits intomesa:mainfrom
quaquel:property_factory
Jan 13, 2026
Merged

Replace PropertyDescriptor with properties#3125
quaquel merged 4 commits intomesa:mainfrom
quaquel:property_factory

Conversation

@quaquel
Copy link
Copy Markdown
Member

@quaquel quaquel commented Jan 12, 2026

This PR replaces the PropertyDescriptor with normal properties using a property factory pattern. This PR follows up on a suggestion from @codebreaker32 in #3080. It roughly halves the access time to a property in a cell.

Implementation details

  1. It uses the property() function to assign getters and setters for a property to a cell.
  2. It also encapsulated layer.data by wrapping it in property. The setter is most critical here because it ensures that self._data is never changed to a different NumPy array, thereby breaking the properties assigned via step 1.
  3. I currently don't add a deleter to the property because I fail to see a use case
  4. A future PR can make layers read-only at the cell level by omitting the setter.

@quaquel quaquel added the performance Release notes label label Jan 12, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 12, 2026

Performance benchmarks:

Model Size Init time [95% CI] Run time [95% CI]
BoltzmannWealth small 🔵 +1.6% [+1.0%, +2.2%] 🔵 -1.3% [-1.4%, -1.1%]
BoltzmannWealth large 🔵 -0.8% [-1.5%, +0.1%] 🔵 -3.0% [-6.5%, +0.8%]
Schelling small 🔵 +2.2% [+1.3%, +3.3%] 🔵 +1.9% [+1.3%, +2.7%]
Schelling large 🔵 +0.1% [-0.1%, +0.3%] 🔵 +2.2% [+0.4%, +3.8%]
WolfSheep small 🔵 -0.5% [-0.9%, -0.1%] 🔵 -2.5% [-2.9%, -2.1%]
WolfSheep large 🔵 -0.4% [-2.9%, +3.8%] 🔵 -1.6% [-2.8%, -0.3%]
BoidFlockers small 🔵 +0.3% [-0.2%, +0.8%] 🔵 -1.2% [-1.4%, -1.0%]
BoidFlockers large 🔵 +0.8% [+0.4%, +1.1%] 🔵 -0.9% [-1.2%, -0.7%]

@EwoutH
Copy link
Copy Markdown
Member

EwoutH commented Jan 13, 2026

Either the current PropertyDescriptor and the new create_property_accessors aren't and won't be used directly by users, right?

@EwoutH EwoutH added the enhancement Release notes label label Jan 13, 2026
@EwoutH EwoutH mentioned this pull request Jan 13, 2026
Closed
@quaquel
Copy link
Copy Markdown
Member Author

quaquel commented Jan 13, 2026

Yes they are purely internal. As you can see, they don't show up in the __all___ for mesa.discrete_space

EwoutH
EwoutH approved these changes Jan 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@EwoutH EwoutH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good.

If you can post any benchmark numbers I think that would be great to have as historical reference.

@quaquel
Copy link
Copy Markdown
Member Author

quaquel commented Jan 13, 2026

See #3113 for some numbers on performance.

@quaquel quaquel merged commit 2b2384a into mesa:main Jan 13, 2026
15 checks passed
@quaquel quaquel mentioned this pull request Jan 27, 2026
Merged
@quaquel quaquel deleted the property_factory branch February 16, 2026 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@EwoutH EwoutH EwoutH approved these changes

Assignees

No one assigned

Labels

enhancement Release notes label performance Release notes label

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@quaquel @EwoutH