Repository navigation
fix: make symbol containers immutable without a setattr hook - #5825
Merged
Merged
Conversation
BradyPlanden
added a commit
that referenced
this pull request
Sep 30, 2026
Contributor
|
Bencher Report| Project | PyBaMM |
| Branch | fix/zero-cost-symbol-immutability |
| Testbed | bare-metal |
🐰 View full continuous benchmarking report in Bencher
⚠️ WARNING: No benchmarks found!
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5825 +/- ##
==========================================
+ Coverage 97.99% 98.08% +0.08%
==========================================
Files 346 346
Lines 35309 35275 -34
==========================================
- Hits 34601 34599 -2
+ Misses 708 676 -32 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Symbol.children, the domain accessors and FunctionParameter.input_names now return immutable tuple subclasses that compare equal to lists, and Symbol.domains returns a read-only copy, so editing them raises TypeError instead of silently desynchronising the symbol's identity. The test suite's mutation guard no longer patches Symbol.__setattr__ (which walked the stack on every slot write); post-construction slot writes are instead rejected by a static AST scan of PyBaMM's code, tests, examples and notebooks. Setting Array/Interpolant.entries_string and calling StateVector.set_evaluation_array are now deprecated in-place updates. Under the guard, expression construction is ~2x faster and parameterisation ~25% faster, recovering most of the test_parameterise regression from #5779.
BradyPlanden
force-pushed
the
fix/zero-cost-symbol-immutability
branch
from
September 30, 2026 14:19
1b39a7e to
91b2439
Compare
BradyPlanden
enabled auto-merge (squash)
September 30, 2026 14:19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The Bencher check is currently failing on
test_parameterise: this report shows 1.01 ms against an 804 µs baseline. The regression comes from #5779. Its test-suite mutation guard, which also covers the benchmarks, hooksSymbol.__setattr__and walks the stack on every slot a constructor writes, andchildrenanddomainbuilt a write-through view on every access.children,domainandinput_namesreturn immutable tuples that compare equal to lists, anddomainsreturns a read-only copy. Editing them raisesTypeErrorinstead of warning.__setattr__hook is replaced by a test that statically rejects post-construction slot writes in PyBaMM's own code.entries_stringand callingset_evaluation_arrayin place now emitSymbolMutationDeprecationWarning.Bencher results
Type of change
Add an entry under
# [Unreleased]in CHANGELOG.md, in one of## Breaking changes,## Deprecated,## Features, or## Bug fixes(include PR number; breaking and deprecation entries need a one-line migration note). Internal-only PRs — refactor, docs, CI, tests — can skip this. See RELEASE.md for the full policy.Important checks:
Please confirm the following before marking the PR as ready for review:
nox -s pre-commitnox -s testsnox -s doctests