Repository navigation
Preserve modifier inputs across direct callbacks and measurements - #2673
Conversation
Aggregate impact on certified data: passBase Checks: 0 broken by this change, 1 failing on both sides, 0 fixed. Moves flagged for review: 0 (thresholds in Notes
Moves flagged for reviewNone. Spending by benefit and service
Caseloads
Standard reform costings (change in gov_balance)
Standard reforms: change in each reform's target
Household net income by decileNo household's net income changes in any year. No change in any year: receipts by tax head; government balance, each component counted once; model totals; household income totals; population; poverty rates; people in poverty; inequality; mean household net income in each side's own deciles. Checks failing on both base and head (1)
Counted-once checkEach of 88 components was raised by £1 per unit in 2026-27, and every head of its totals was read household by household: its own head must rise by exactly £1, no other tax head may move, no other benefit may rise, and the model's own total must move by what its heads moved. A head that breaks a rule is measured again twice: with every other head held at its baseline value, and with every other head held at its value after the first £1 while the component rises again. It fails where either still shows the response. 0 failed; 0 were skipped.
Provenance
Dataset: Receipts by tax head: head levels
Spending by benefit and service: head levels
Caseloads: head levels
Government balance, each component counted once: head levels
Model totals: head levels
Household income totals: head levels
Population: head levels
Poverty rates: head levels
People in poverty: head levels
Inequality: head levels
Mean household net income in each side's own deciles: head levels
Standard reform costings (change in gov_balance): head levels
Standard reforms: change in each reform's target: head levels
|
151ad0b to
422885e
Compare
|
Round 1 at Earlier points from our issue comments
New findings
What I checked and found fine
What I ran: our 🤖 Generated with Claude Code |
422885e to
bb05334
Compare
|
Changes responding to the findings in the review at
Final native candidate is |
|
Round 2 at Round 1 findings
New findings
Nothing at must or should. Timings (base
Every result is identical on both sides: What I ran: fresh worktrees and fresh What I did not do: re-run the new tests on base (round 1 showed 38 of 66 failing on Verdict: ready to merge. CI is green (34 pass, 1 skipping is 🤖 Generated with Claude Code |
vahid-ahmadi
left a comment
There was a problem hiding this comment.
Approving at bb05334; see the comment above.
Fixes #2666
Fixes #2667
A modifier can read an output and replace it with
set_inputduring construction, throughScenario.apply, or directly on a built simulation. Later reads, dynamics, marginal rates and exports retain the surviving explicit inputs visible to that simulation, including input-only variables absent at construction, overwrites and explicit zero. Calculated intermediates remain live; stores on another clone or branch do not become parent inputs.reset_calculationsnow retains every surviving explicit input, including ordinary post-constructionset_inputcalls outside any modifier. A reset alone no longer undoes those overrides. To restore a formula or default, calldelete_arrays(variable, period), then reset or recalculate. Every top-levelset_inputon a built simulation queues dependent-cache cleanup before its next outermost read, including simulations with no modifier or scenario. The guide and changelog disclose both general caller-visible changes.The constructor and
apply_reformpreserve their existing success-only own-write naming and input-list isolation. Shared consumers inspect actual stored-input metadata. Formula-owned writes to a child branch while any ancestor is calculating use Core's selective invalidation, preserving unrelated computed values. Pension Credit and the UC housing calculation therefore share their two earnings-tax counterfactuals in either calculation order. Idle user-branch edits still refresh outputs to the cold-construction benchmark. Clone calculation depth and separate baselines retain their existing lifecycle.All three marginal-rate formulas take fresh scratch names distinct from existing registrations and the current branch. Identity-guarded
finallycleanup detaches only the branch each formula owns, including on failure. Existing named user branches keep their objects and inputs. The CGT step, masks, held responses and all tax arithmetic remain unchanged. After a successful construction-time modifier, the completed cleanup clears its pending flag so the first read does not drop computed values twice; callback or cleanup failure retains invalidation for a subsequent retry. Default construction now also drains pending framework input invalidation before cloning its baseline, then clears the flag only after successful cleanup. The default baseline therefore receives coherent explicit inputs and cold calculations rather than copying pending work for another first-read drain. Independent Scenario baselines and user callbacks keep their existing order. This does not suppress framework writes, change the active-ancestor rule or discard queued external edits.The declared minimum Core version is now 3.33.0, matching the public
drop_computed_arraysAPI introduced in its tagged changelog. The lock's resolved Core remains 3.33.2; other dependency versions, sources and hashes are unchanged. The guide names this minimum and explains the explicit cache boundary after direct parameter-tree updates.Merged policyengine.py #718, which closes #717, supplies the wrapper boundaries. Its actual merge
ba50a5f8104a906a6080d01af2b5883a27b3f108uses_apply_stepto clear computed arrays infinallyafter the Policy callback, late Dynamic parameter application and Dynamic callback, preserving their order. These boundaries are included in published wrapper 6.11.0, tag commitb4158177d8e4d52657cb04b2ec022044688aa841. This country model needs no wrapper dependency or copy of orchestration code.Historical public real-wrapper checks recorded in #717 use two country versions. The released-version checks use UK 2.132.6/Core 3.33.3: a local wrapper prototype restores a specified £1,000 annual Child Benefit increase and passes 6 supported cases. Separate historical supplemental candidate measurements use native source corresponding to the earlier published commit
422885e993a6381be711397cb8dfb2be6496dd4ewith Core 3.33.3. On that candidate source the prototype replaces a stale zero response with the negative cold-matching response (approximately −£27.48 annual employment income) and passes 6 supplemental integration cases. These are public synthetic household checks; they are not measurements of the final native commit or a population estimate.UK 2.132.6 has a separate named-baseline-branch limitation: its nonzero labour-supply control fails even cold. The historical candidate comparison does not attribute every intervening native baseline change to this PR. Wrapper 6.11.0 contains #718, but its
uk,modelsanddevextras still pinpolicyengine-uk==2.132.6andpolicyengine-core==3.33.3. The remaining shipping bridge is to merge and release this native UK repair, then advance the wrapper UK pin or installed country model to a suitable native release; a change to the pinned extras needs its own wrapper packaging release. The earlier 6.10.2 installed metadata identifies historical test environments and does not describe the latest wrapper release.The current formatted candidate is
bb05334596c07d206113caab17d9bdc68b64726d, based on actual main2b0b74f6185a69b8b6ded4eb028cf9240bbb9a79, which includes the independently reviewed Housing Benefit special-disregard update. Its source project version is UK 2.162.0; the frozen native Python 3.13.14/Core 3.33.2 environment still reports installed UK metadata 2.161.2. No environment sync or other dependency change was made.Historical R4 evidence remains 1,021 distinct native passes/one existing allowed credential skip, using a latest-per-ID ledger rather than repeated-result sums. The three new default-baseline tests first failed causally on unchanged
37cb2390: pending work crossed the clone boundary, first reads ran two additional actual drains, and an injected terminal-drain failure did not raise because that drain was absent. After the narrow constructor fix, the affected seven-module suite passed 148 cases with no failures, errors or skips on the previous112f71bdbase. Those are explicitly previous-base executions; no whole-suite claim is made for this final candidate.Fresh validation on this exact
bb053345/2b0b74f6source passed 37 Python cases (313.614 seconds) and 44 Housing Benefit YAML cases (5.683 seconds). The Python selection covers all 29 affected HB/CTR properties and 8 constructor, PC and after-load controls. A separate 8-case Core 3.33.0 gate passed (31.089 seconds), and 6 actual merged #718 integration cases passed on Core 3.33.3 (43.862 seconds). All four selections had zero failures, errors, skips or xfails and executed every selected ID once per environment. These elapsed values describe CI execution, not the performance comparison. The fresh selections are reported separately from the historical 148-case suite and 1,021-pass ledger; they are not summed into a new full-suite claim. The historical allowlisted skip ispolicyengine_uk/tests/test_scenario_baseline_parameters.py::test_before_load_spending_scenario_constructs_on_the_dataset.All fresh gates use Python 3.13.14 and import the same final UK source project 2.162.0. Installed editable UK metadata remains 2.161.2 in the native environment, 2.161.1 in the minimum-Core environment and 2.158.2 in the wrapper environment. The wrapper environment reports installed metadata 6.10.2 but executes the immutable actual merged #718 source
ba50a5f8, with its model module verified by hash; this does not identify a wrapper release. Earlier R4 compatibility executions (17 cases on Core 3.33.0, the real public constructor/reset/drop/MTR probe, six actual merged-wrapper cases and the public NI EPG probe) remain historical evidence and are not presented as fresh executions of the R5 change. The current published wrapper release is 6.11.0, as described above.The new seven-case RED result was five causal failures and two passing failure controls. Four failures expose CGT collision/exception ownership, and one exposes duplicate after-load cleanup. Original 30 Scenario cases remain unchanged. Existing read-only cache, held response, static marginal-rate, tax/WFP inequality, tolerance and pinned property examples remain intact. The byte-unchanged
policyengine_uk/tests/test_pension_credit_tax_branch_reuse.py::test_full_calculation_reuses_two_earnings_tax_counterfactualsretains its exact two-call assertion in both orders.The new constructor regressions are
policyengine_uk/tests/test_scenario_modifier_inputs.py::test_default_baseline_clones_after_framework_input_drain,policyengine_uk/tests/test_scenario_modifier_inputs.py::test_default_constructor_first_reads_do_not_repeat_framework_drain, andpolicyengine_uk/tests/test_scenario_modifier_inputs.py::test_default_baseline_drain_failure_retains_pending_inputs. They retain positive cold-reference PC/HNI, explicit inputs, the unchanged two-counterfactual contract and failure recovery.The final public synthetic benchmark compares immutable archives of actual base
2b0b74f6and candidatebb053345, with identical parameter-file/directory topology and unchanged natural GC settings. P1 reads Pension Credit before household net income; P2 reads household net income first; M exercises the marginal-rate and held-response fixture. Each snapshot/case has one warmup, three unprofiled repeats and one profiled observation. Thirty distinct primary constructions, two held controls and six constructor/query profile pairs completed across two sessions, with no valid case repeated. Every direct economic and held output difference was zero, and positive/cold-reference checks passed.Candidate/base median cost ratios are:
The matched CPU medians in seconds (minimum–maximum of the three repeats) are P1 base 1.483190 (1.473230–1.682783) versus candidate 1.571059 (1.552682–1.652038); P2 base 1.553876 (1.486268–1.604590) versus candidate 1.517760 (1.447520–1.777766); and M base 4.308435 (3.848448–4.500409) versus candidate 4.249156 (4.248524–4.291988). These are medians of each matched construction-plus-query observation, not sums of separate medians. End-to-end CPU ratios including ordinary release are 1.059139, 0.976767 and 0.986223 respectively. P1 retains a measured 5.9% total CPU increase; the observed P2/M medians are lower in these three-repeat public fixtures. This is not a zero-overhead claim or a population timing estimate.
Both Pension Credit cases preserve exactly two tax counterfactuals, three clones/branches and identical Core calculation counts in each base/candidate pair. Their profiled query full-cache drops are zero, while candidate construction performs eight drops. M preserves its five counters and 22 clones/branches, but its profiled query performs three cleanup-helper drops versus two on the base, with 16 during candidate construction. The zero-query-drain result therefore applies only to P1/P2.
The initial benchmark session
78047remains an overall FAIL: its 20,000-event telemetry cap overflowed only in the untimed baseline-M held-query GC tail, after its timed observations and economic comparator completed. The retained evidence includes P1/P2 and baseline M. A separate authorized candidate-M-only session58600passed in 34.674123 seconds, completing only the missing five primary constructions and one held control. All timed GC records are complete; the untimed baseline-M held-query GC tail remains incomplete. The completion recorder appended 20,042 events below its 100,000 cap. Default thresholds[2000, 10, 10], enabled state and prior callbacks were restored. This combined evidence is not represented as an uninterrupted full-run PASS, and no GC-causality claim or causal timing subtraction is made. The inherited aggregate workflow allows 120 minutes; a completed certified published-head report and reviewer acceptance remain separate readiness gates.The changed guide passed one HTML build with notebook execution disabled; its source bytes remain unchanged. The single R5 formatter preserved all 1,794 reviewed CPython 3.13.14 ASTs, with only byte formatting in the Scenario test file. The subsequent conflict-free rebase preserved all 13 nonmetadata PR path bytes and all 10 nonmetadata Housing Benefit update path bytes, with the exact country-version/Core-floor union in the two manifests. The full tracked source aggregate is
2d983d36e505e3026b6d95bf54c4a4428a8fa831255039ffff443bed88488150. Full published-head CI, including the completed certified aggregate report and review acceptance, remains a separate readiness requirement.Reproducer: policyengine_uk/tests/test_direct_modifier_inputs.py::test_direct_callback_response_matches_parameter_benchmark
Reproducer: policyengine_uk/tests/test_direct_modifier_inputs.py::test_direct_writes_survive_reset_marginal_rates_and_export
Reproducer: policyengine_uk/tests/test_direct_modifier_inputs.py::test_internal_nested_branch_keeps_unrelated_tax_counterfactual_cache
Reproducer: policyengine_uk/tests/test_direct_modifier_inputs.py::test_idle_nested_user_branch_edit_matches_cold_parameter_benchmark
Reproducer: policyengine_uk/tests/test_scenario_modifier_inputs.py::test_modifier_recalculates_cached_marginal_rate_branches
Reproducer: policyengine_uk/tests/code_health/test_cached_arrays_not_written_in_place.py::test_branching_runs_with_read_only_cache
Reproducer: policyengine_uk/tests/test_capital_gains_response_default.py::test_pay_rise_branches_hold_the_realisation_response
Reproducer: policyengine_uk/tests/test_property_income_properties.py::test_choice_is_never_worse_than_either_route
Reproducer: policyengine_uk/tests/test_scenario_modifier_inputs.py::test_capital_gains_rate_preserves_preoccupied_user_branch
Reproducer: policyengine_uk/tests/test_scenario_modifier_inputs.py::test_capital_gains_rate_failure_preserves_user_branch_and_detaches_owned_scratch
Reproducer: policyengine_uk/tests/test_scenario_modifier_inputs.py::test_after_load_modifier_clears_calculated_values_once
Reproducer: policyengine_uk/tests/test_scenario_modifier_inputs.py::test_after_load_cleanup_failure_keeps_pending_write
Reproducer: policyengine_uk/tests/test_reform_inputs_survive_reset.py::test_a_reform_names_no_input_it_did_not_store
axiom: n/a: simulation input retention, cache invalidation and marginal-rate scratch-branch lifecycle; no statutory tax or benefit rule changes.