Repository navigation
Share nested metadata values between parameter clones - #631
Conversation
Parameter, ParameterAtInstant, ParameterNode and ParameterScale clones deep-copied their metadata. On policyengine-us one tree clone makes 1,257,541 ParameterAtInstant clones, and those deep copies were most of the cost of TaxBenefitSystem.clone(). Each clone now gets its own metadata dict (copy.copy, so None and dict subclasses behave as before) holding the source's values. Top-level edits stay on their side; a nested value must be replaced, not edited in place. interpolate_parameters was the only writer that edited a nested value after a clone exists (TaxBenefitSystem.__init__ clones the tree into parameters.baseline before interpolating), so it now replaces the interpolation dict it marks complete. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
propagate_parameter_metadata hands each descendant the node's own interpolation dict, so marking it complete in place skipped every child after the first. Replacing the dict fixes that too; no PolicyEngine country model uses propagated interpolation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replacing the parameter's `interpolation` dict left the propagating node's dict unmarked, so `TaxBenefitSystem.__init__`'s final `propagate_parameter_metadata` overwrote the parameter's marked copy and a second `interpolate_parameters` added every entry again. Completion belongs to a parameter's values, while the `interpolation` dict is shared by its clones and by every parameter under a propagating node. So `Parameter.interpolated` now holds it, and `interpolate_parameters` writes no metadata. Add the constructor sequence as a test and as a property, an `interpolation` node to the shared fixture, and a differential against the in-place version's values. Correct the docs on which dated values hold their parameter's metadata. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An `__init__` assignment left no attribute on policyengine-uk's `LegacyNIParameter` and other subclasses that skip `Parameter.__init__`, or on a tree pickled before the attribute existed, so interpolating one raised AttributeError. A class default reads the same everywhere and adds nothing to a parameter that never interpolates. Document how to interpolate a parameter again after changing its `interpolation`, which on master took only a new dict, and let the generated edits set a monthly interval. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A tree pickled by an older Core carries `completed`, and without this read a second `interpolate_parameters` on it added every entry again. Core still writes no metadata. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Landing audit at Reviews. GPT-6.1 Sol reviewed five rounds, each in its own worktree:
Core gate ( This head. Core suite 2,826 passed, 3 xfailed, 4 skipped. Country template 39 passed. CI 19 of 19, MERGEABLE, CLEAN. policyengine-uk Judgment. No public claim, money, legal, people or outside send. Model outputs stay the same: digests of policyengine-us and policyengine-uk match master at |
Relates to #621 and #618 (does not change the cache design either discusses).
Problem
Parameter.clone,ParameterAtInstant.clone,ParameterNode.cloneandParameterScale.cloneeach calledcopy.deepcopy(self.metadata). policyengine-us's tree has 22,796 nodes, 959 scales with 8,209 brackets, 124,946 parameters and 1,338,663 dated values, so oneclone()of it ran about 1.5 million metadata deep copies.TaxBenefitSystem.clone()andReform.__init__each clone the tree, so every reform build pays this, as do policyengine-us's behavioural-response measurements andSimulation.clone(), which reachTaxBenefitSystem.clone().Change
copy.copy, holding the same values as the source's. Setting, updating, popping or replacing a key changes only the object that receives it. A nested value (areferencelist, anupratingdict) stays one object for both until an edit replaces its key.copy.copykeeps the old behaviour forNonemetadata (staysNone) and for dict subclasses (keep their type).Variable.clonealready copied metadata this way (dict(self.metadata)).interpolate_parametersrecords its work on the parameter. It wrotemetadata["interpolation"]["completed"] = Truein place, into a dict that clones now share.TaxBenefitSystem.__init__clones the tree intoparameters.baselinebefore it interpolates the whole root, so that write would have skipped the second copy reached. Completion belongs to a parameter's values, while its clones and every parameter under a propagating node share theinterpolationdict, and propagating again replaces each parameter's reference to it. SoParameter.interpolated, a class default ofFalsethat the function sets on the instance and clones copy, now holds completion, andinterpolate_parameterswrites no metadata. The class default also covers subclasses that skipParameter.__init__, such as policyengine-uk'sLegacyNIParameter, and trees pickled by an older Core. Three effects, none of which moves a model value, because no PolicyEngine country model setsinterpolationon any parameter (see the audit):propagate_parameter_metadatagives each descendant the node's owninterpolationdict, so the marker on it skipped every child after the first. On master, a node withpropagate_metadata_to_children: trueandinterpolation: {interval: year}overa(100 → 140) andb(200 → 280) givesa(2016-06-01) == 110butb(2016-06-01) == 200; it now gives 220.completed.interpolate_parametersstill honours a truecompletedalready in the metadata, so a tree pickled by an older Core doesn't interpolate twice. In every repository searched, only that function readsinterpolation, and no YAML sets it.interpolationno longer restarts a parameter. On master a new dict interpolated the parameter again. Now that also takesparameter.interpolated = False, and the new dict must not carry a truecompleted, as the function's docstring says.docs/usage/parameters.mdstates the rule: replace a key to change a nested value. Within one tree this already held: a dated value written in dictionary form holds its parameter's metadata values (a scalar entry has none), andpropagate_parameter_metadatagives descendants the node's values.Invariants
[k] =,update,setdefault,pop,del,clear, replacing nested values),Parameter.update, assigning or removing a dated value,propagate_parameter_metadataandinterpolate_parameters.test_a_clone_shares_nested_metadata_values_with_its_source.TaxBenefitSystem.__init__'s sequence (propagate, interpolate, propagate), anotherinterpolate_parameterschanges nothing: each parameter interpolates once, whichever object gave itinterpolation.Audit of metadata writers
Searched Core, policyengine-us, -uk (plus PolicyEngine/policyengine-uk#2756's diff), -canada, -il, -ng, policyengine.py, policyengine-api, -api-v2 and -household-api for every write to parameter, value, node or scale metadata, and read each line that reads one for locals that alias a nested value. First pass on 2026-10-24, repeated on each repository's default branch on 2026-10-30.
interpolate_parametersedited a nested value after a clone can exist; it now writes no metadata.propagate_parameter_metadata(update, top-level) also runs after the baseline clone and needs no change.ParameterScale.propagate_unitsandParameterScaleBracket.propagate_upratingwrite top-level keys, inParameterScale.__init__and, forpropagate_units, also from policyengine-api'scountry.py.normalize_uprating_metadatareturns a dict argument unchanged, and the uprating code that receives it only reads it.homogenize_parameter_nodereads thebreakdownlist and recurses on a slice.system.pyclones the OASDI cap intogov.contrib.ssa.owners_draw.current_law_cap, then callsupdateandpop("uprating", None)on the clone's metadata. Itsspm.pytree sharing sets node attributes and caches, not metadata.setdefault/pop/updateindated_reforms.py,parameters.py,create_threshold_indexation.py,reindex_benefit_cap.py;ni_parameter_scales.pyswapsmetadata["uprating"]for a new dict and restores it;create_savings_credit_threshold.pysetsupratingon a clone. Derive economic assumption index horizon from the growth series policyengine-uk#2756 adds one read.interpolation: no YAML in the five country repositories has that key, and no Python in them names it. So the interpolation change has nothing to act on, although policyengine-us has 880 YAML files withpropagate_metadata_to_children: trueand policyengine-uk 65. Digests confirm it. Each of policyengine-us (under two hash seeds) and policyengine-uk, built on master and on this branch, gives the same SHA-256 over every object's type, name, metadata, dated values and child links, walked by real child keys and bracket positions, in the tree as built, in policyengine-uk'sbaselineand in aclone(): 156,910 objects, 124,946 parameters and 1,338,663 values in policyengine-us; 6,480 objects, 4,626 parameters and 73,659 values in policyengine-uk. A second digest over whatget_descendantsyields, which includes policyengine-uk's NI discovery views, also matches. A moved value, an edited nested metadata entry and a swap of two children each change the digest.The two clone-then-edit writers above (policyengine-us's cap, policyengine-uk's savings credit threshold) are why each clone still gets its own dict: sharing the dict outright carries those edits back to the source parameter. With that mutant, policyengine-us's real OASDI cap loses
upratingand takes the label "Current-law OASDI cap preserved for owners' draw"; on this branch it keeps both.Benchmark
TaxBenefitSystem.clone()on policyengine-usmain(bdd08b8d69), CPython 3.14.7, macOS arm64, Core master (33dc9bb) against this branch. One venv ran both sides, withPYTHONPATHchoosing the Core, so no other package differs. Four rounds in ABBA order, fresh processes, five timings per measurement per round: 20 samples a side, medians below. Times are CPU time (time.process_time) rather than wall time, because the host stayed busy (load average 46 to 106 on 18 cores when each process started); only the full-collection row uses wall time. In each of the three clone timings, every master sample exceeds every branch sample.system.parameters.clone(), GC offsystem.parameters.clone(), GC onsystem.clone()(policyengine-us'sclone_spm_system, which callsTaxBenefitSystem.clone), GC onparameters.clone()parameters.clone()system.clone()sgc.collect()with three clones aliveOf the 0.54 M fewer tracked objects per tree copy, 0.39 M are dicts and 0.15 M lists. The tree copy takes 2.10 s with the collector on and 0.38 s with it off, so collection passes during the copy account for most of the time that remains.
The table comes from
f99fd71bf. Later commits add one class attribute and otherwise change onlyinterpolate_parameters, which no country model's parameters reach. One more interleaved round, at4db4c7cae(one docstring from this head), agrees: 2.37 → 0.39 s with the collector off, 5.31 → 1.88 s with it on,system.clone()5.59 → 2.26 s, 722 → 632 MB kept per tree copy, 1.37 → 1.20 GB kept by three clones, peak 3.90 → 3.61 GB, and 6.69 M → 6.14 M tracked objects.Build time does not change.
CountryTaxBenefitSystem(), one venv, four ABBA rounds of three builds per process, mean CPU seconds by build order: collector off, master 6.70, 6.77, 6.76 and this PR 6.85, 6.65, 6.56; collector on, master 8.88, 10.30, 9.83 and this PR 9.42, 9.08, 10.22. The collector-on differences change sign from one build to the next and the collector-off means sit within 0.2 s of each other, which points to where collection passes land rather than to the code, though the script records no collection events.policyengine-uk. The US + core hub's interleaved probe, on the tree PolicyEngine/policyengine-uk#2756 builds (91,410 processed entries), three CPU-time runs per side at this PR's
449d2d4, which has the same clone code as this head: processed tree copy 309–314 ms → 183–189 ms, new system 542–551 ms → 366–372 ms, memory per kept system 50.7–50.9 MB → 48.0–48.1 MB.Script and raw results:
bench_clone.py(run one variant per process;installedmeasures the code as installed), below.bench_clone.py
policyengine-us test time
policyengine_us/tests/core/test_behavioral_response_measurement_copies.py(16 tests), which these copies dominate. User plus system CPU and peak RSS from/usr/bin/time -l, one venv, policyengine-usbdd08b8d69, two rounds per cell with the order of the sides swapped between rounds. The ranges span the two rounds; the host stayed busy, and master's first default run (862 s) sits well above its second, so read them as ranges rather than a percentage.Tests
tests/core/parameters/test_parameter_metadata_clone.py(110 cases): each metadata edit (set, replace nested,update,setdefault,pop,del,clear) on each kind of object (node, parameter, dated value, scale, bracket, bracket component), on either side of a clone; nested values shared;Noneand dict-subclass metadata copied ascopy.copycopies them; a tree holding its own clone interpolates both (theTaxBenefitSystem.__init__shape), including a realTaxBenefitSystemwith aparameters_dir; on a real system whose node propagatesinterpolation, a secondinterpolate_parameterschanges nothing; every child a node propagatesinterpolationto interpolates; interpolating writes no metadata; clearing the flag interpolates a parameter again; acompletedkey already in the metadata stops interpolation; a parameter that skipped__init__interpolates; interpolating or propagating one side leaves the other alone; acloneordeepcopyof an interpolated tree stays interpolated;TaxBenefitSystem.clone()and aReformthat edits metadata leave their source alone; cloning the country template's tree callscopy.deepcopyzero times.tests/core/parameters/test_parameter_metadata_clone_property.py(Hypothesis, 300 examples each, derandomized): invariant 1 over clone chains; invariant 2 againstdeep_clone, a copy of master's clone methods; the two-tree case against the untouched side's own snapshot; invariant 4 over clone chains; and the valuesinterpolate_parametersgives againstinterpolate_in_place, a copy of master's function, on parameters that each have their owninterpolationdict. The shared fixture has a node that propagatesinterpolation, and the generated edits can setinterpolationon any object.getattr, 14 of the 115 tests fail, all on behaviour this PR changes: 12 example tests on sharing, zero deep copies and how interpolation records its work, and the two properties that compare a tree with a fresh one. Those two fail for one reason, minimal case a singleinterpolate: on master a fresh tree's dated value shows"completed": Trueand a clone's does not, because the clone deep-copied that value'sinterpolationapart from its parameter's. With that key left out of the snapshot, all 5 properties pass on master's code. The other 98 example tests and 3 properties pass unchanged.values_list, dated values not cloned, and master's in-place interpolation. Invariant 4's property fails on an unrecorded interpolation and on this PR's first version, which replaced the parameter's dict. A clone that forgetsinterpolatedfails the chain and differential properties. A flag assigned only in__init__and acompletedkey ignored each fail the one example test that pins them.Checks run
At this head,
21a8c5f9a, on master33dc9bb36:cbe322b51c: 782 passed.7ed52038a, the 36 test files that reach its metadata writers: 2,487 passed and 61 skipped on master and on this branch, with no test changing status between them. A coverage run of its NI tests (128 passed on this branch) shows they executeni_parameter_scales.py's two metadata writes.setdefaults, the NI view with its uprating references, the savings credit threshold projection), run through a statutory cache, a processed cache and a fresh system, changes no tree it was not made on, and gives the same income and the same reformed tree on master and on this branch. Two controls show the probe can see a leak: a top-level edit leaks only under the shared-dict mutant, and a nested in-place edit leaks on this branch as invariant 3 says it does.The digests and the last benchmark round ran at
4db4c7cae, one docstring from this head.Follow-ups (not in this PR)
NICompatibilityNode.get_descendantsyields a fresh view on each walk, so a flag set on a view goes with it, and a second walk would interpolate the target again. On master the marker reached the target through the shared dict. This takes two things policyengine-uk doesn't do: settinginterpolationand callinginterpolate_parameterson that tree.Reform.modify_parametersstill callscopy.deepcopyon the whole baseline tree.Reform.__init__already clones it, so that call pays the deep copy this PR removes elsewhere.ParameterAtInstant's metadata after construction in any repository searched, but nothing enforces it, so this PR still copies each dict. Measured on policyengine-us (one round, same script,dict_shared_valuesvariant), sharing them would keep 545 MB per tree copy instead of 632 MB, add 4.8 M tracked objects instead of 6.1 M, and copy the tree in 1.88 s instead of 2.10 s with the collector on.NICompatibilityNode.cloneoverridesParameterNode.cloneand still deep-copies its own metadata.axiom: n/a: infra (Core parameter-tree copying; no policy change)
🤖 Generated with Claude Code