Repository navigation
Conversation
Building every descendant in the constructor made each instant a system is asked about cost a copy of the whole parameter tree, kept for the system's lifetime. For policyengine-us that is about 34,000 node objects, 16-33 MB and several seconds per instant, per tax-benefit system. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-coherent Review of 404a3a3 found five regressions against the up-front build: - concurrent first reads could raise KeyError/AttributeError or resolve a child twice: first resolution now runs under a module RLock; - children named like the new state or helpers (_node, _resolved, _resolve, ...) read as internal objects: state is name-mangled, helpers are module functions, and children named like the node's own attributes resolve at construction, as before; - a shallow copy and its original could resolve a scale twice: both now read one grow-only resolved map; - a child with no value at the instant was re-resolved on every read: its absence is cached; - deepcopy/pickle of a traced tree's raw node recursed through TracingParameterNodeAtInstant.__getattr__, which now raises AttributeError for its own unset fields. A node also drops its parameter node once every child has been read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Measurements of this PR combined with #569 on the two policyengine-us CI batches that are failing on 16 GB runners: see #569 (comment). Together they take the NY/OH/OK batch from 16.3 GB (3.32.11) to 7.0 GB and the USDA batch from 17.1 GB to 6.4 GB peak footprint. |
…ases Round-2 review of a90b524 found reads that still differed from the up-front build. A node now keeps all of its own state in one _LazyChildren object under a single reserved name (a parameter of that name is rejected): - which children a node has, and their order, are fixed at construction (a child deleted from the tree later keeps its place); - completion counts only those children, so add_child extras can no longer send the first read into recursive completion; - resolution results and add_child entries are kept apart: items are shared by shallow copies, attributes belong to each copy, as before; - a copy installs what it reads as attributes (no lock on warm reads); - children named like attributes of the node's runtime type (dunders and subclass methods included) resolve at construction; a subclass overriding add_child gets the up-front build through it; - a reentrant read keeps the first recorded child; - nodes pickled by the up-front build load (__setstate__). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 7, 2026
This branch has not been deployed
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.
Draft: measurements on policyengine-us and policyengine-uk are running; this description gets the before/after numbers when they finish.
Problem
ParameterNodeAtInstant.__init__built every descendant up front. Every instant a tax-benefit system is asked about therefore materialised the whole parameter tree, andParameterNode._at_instant_cache(plusTaxBenefitSystem._parameters_at_instant_cache) keeps it for the system's lifetime.Measured on policyengine-us (132,396 parameter descendants) on master b78b0ba:
A YAML run touches every year it tests and every month of a monthly variable, so the shared system and each reform system all pay this many times over. It was the third cause of the 118 GB
policyengine-core testruns on 2026-10-02, next to the test-runner causes fixed in #569; the two PRs are independent.Change
A node at instant resolves a child the first time it is read (
__getattr__,__getitem__) and keeps it. It resolves all of its direct children only when something needs all of them:_children, iteration,repr,dir, vectorial indexing. A resolved child is a plain instance attribute, so hot reads cost what they did.Invariants (tested)
_children,dir,repr, vectorial indexing, names that do not exist), every read returns what the up-front build held, and a node read in full equals the up-front build, including child order.test_parameter_node_at_instant_property.py(Hypothesis) checks this against a reference that walks theParameterNodetree directly, and it passes on master too.ParameterNotFoundError/KeyErroras before.test_a_child_not_read_before_an_update_reads_the_update)Tests
uv run pytest tests/core/parameters tests/core/parameters_fancy_indexing tests/core/test_parameters.py tests/core/test_tracers.py tests/core/test_reforms.py🤖 Generated with Claude Code