Correct UC measurement contracts and calibration iterate selection - #883
Correct UC measurement contracts and calibration iterate selection#883juaristi22 wants to merge 6 commits into
Conversation
vahid-ahmadi
left a comment
There was a problem hiding this comment.
Review pass at 9d13d1dd (Claude Code, high effort; worktree checkout, the PR's own diff against #879's head eb7ca8a5 — 47 files, +3,778/−705 — read alongside the companion policyengine-uk #1850, which I reviewed this morning; without the engine: the in-scope build, calibrate and data suites all pass with only requires_uk and missing-feed skips, but adding microcosm-graph/tests/test_acceptance_h_parity.py gives exactly one failure; ruff check and ruff format clean on all 31 changed Python files; both census tools, the coverage manifest and ci_test_groups --verify current. No hosted CI runs on this PR because the workflow triggers only on PRs to main.)
The measurement work is careful
The shared claimant helper (uc_relationships.py:52-59) derives claimants as head, parent, or every member of a childless unit, refuses a unit with other than one or two claimants, and is now the single couple definition for the redraw predictor (uc_reporter_redraw.py:58, 73, 528) and the capital-donor stage (uc_capital_coherence.py:206), with is_married dropped from the required inputs and the stale contract refused by test — that closes the calibration/redraw divergence I raised on #879. The TCL measures group on person_benunit_id and reindex on benunit ids so nothing leaks across units or drops; the post-cutoff proxy never reads the engine's denial flag; the caveats are carried in the contract and surfaced through materialisation. The band cap is a declared band_upper_bound: 2500 with an inclusive flag, materialised as nextafter(30000, inf) so £30,000.00 is in and £30,000.01 is out, and invalid declarations raise. GB is a declared household region predicate projected to benunit grain with unmatched links raising. The nine-month requirement is a per-target source_months declaration validated post-compile — exactly one fact per declared month, refusing missing, extra, duplicate or wrong-year — with a failure surfacing as unsupported and the national calibration raising; compiled values are unchanged (111 metadata lines added, zero value lines moved). The helper is hashed into the three consuming stages' kernel identities and the mutation test proves it. All of that checks out.
Findings
1. Blocking — the calibrate kernel's parity pin is stale, so test_h1_kernel_parity fails. kernels.py:178-181 hashes solve.py's source; this PR edits solve.py but leaves packages/microcosm-graph/tests/fixtures/parity/kernels/calibrate/pins.json at a7a0330f…, and the assertion at test_acceptance_h_parity.py:177 sees 142dc3bf…. Swapping the base solve.py back in restores the match, so the cause is the solver edit. Because the hash assert fires first, the calibrate kernel's direct.csv byte-parity check never runs, which means the conserve-mass path the PR says is unchanged is currently unguarded. Regenerate the pin and let the parity check run.
2. Blocking — person.is_uc_claimant is exported but on no allow-list, so the release-cut export gate refuses it. The column is added to frs_spine.py's outputs and the graph root, but uk_export_surface.allowed_extra_columns in gates.json and UK_ALLOWED_EXTRA_EXPORT_COLUMNS in terminal_gates.py do not carry it (they carry person.is_parent), and the eFRS reference surface does not either. uk_export_surface_gate computes unexpected = (candidate − reference) − allowed_extras; calling it with the reference plus this column returns passed=False naming person.is_uc_claimant. The body's line about the frozen coverage candidate refers to the coverage manifest, not this gate. Either allow-list the column (it is the whole point of the companion #1850) or explain why it should not ship.
3. Should-fix — the best-iterate solver change reaches the UK national production path with no byte-identity guard. solve.py:842 applies retention iff gates is None and not conserve_mass and l2_lambda == 0, and gates exist only with l0_lambda > 0 or target records. The national doctrine is mass_rule = "free" and l0_lambda = 0.0, and national_calibration.py:128-142 passes neither l2_lambda nor target records — so the national solve is exactly the changed path, and so is the local rowwise default. The three new solver tests are positive cases; nothing pins that conserve, L2 or L0 runs are byte-identical to before, and nothing measures what the change does to certified national weights. This is a generic calibrate-package behaviour change riding in a UK measurement PR; split it out, or at minimum add the byte-identity fixtures for the untouched paths and say in the body that free-mass national weights can move.
4. Should-fix — the helper and #1850's fallback disagree on the ambiguous cases, and results depend on whether the column ships. Microcosm: head, parent, or childless-unit member, no age test. Engine fallback: head, parent, or adult-and-not-qualifying-young-person. Divergences: a 17-year-old cohabiting partner in a childless unit is a claimant here (the PR's own test_uk_frs_spine.py:918-951 asserts it) but not in the fallback; a 19-year-old dependent past the terminal date is a claimant in the fallback but not here. With finding 2 in force the column cannot reach a certified export, so the engine would use its fallback and compute UC on a different family from the one calibration measured. The contract between the two should be stated once, in both PRs; I raised the mirror of this on #1850.
Questions
- The engine-backed checks used a 2.94.0 development runtime with #1850 applied while the lock pins released 2.94.0, which lacks
is_uc_claimant;efrs_parity_known_gaps.json's version bump is a one-line hand edit. Nothing here is reproducible against the locked dependency until #1850 releases and the lock moves — worth saying in the body. - The redraw's landing rule (
_claimant_rows, eldest working-age adult byis_SP_age) is not unified with the helper; couple status is. Presumably intentional; state it.
Nits
The changelog fragment holds three entries; the composition error text changed.
The contract corrections (A through E, G, H) are the substance and they are sound. 1 and 2 are objective — a failing parity test and a gate that refuses the new column — so request-changes until they land; 3 and 4 are the two design points I would want settled before this reaches a certified build.
eb7ca8a to
1f7d676
Compare
9d13d1d to
1f85b82
Compare
|
Thanks, Vahid. Addressed your review through latest commit
The new rebase preserves #881's policy-year rules alongside these fixes. Only three generated artifacts required refresh: the source-coverage manifest, UK specification assertion and H2 graph stage-contract hashes. Combined integration passed 388 tests, with one existing licensed-SPI-donor test skipped because its input is not staged locally. Formatting, lint, inventory, the official-main ratchet and independent verification passed. Earlier calibrate (225 passes), H1 and portable-control evidence is retained only for unchanged source/input bytes and locked dependencies; it was not rerun for this rebase. CI at |
|
Adding the diagnostic results behind this PR, separately from the implementation/review reply. The repairs address demonstrated relationship, measurement and optimizer defects. They do not yet close the UC population miss. Zero-support cells are already acknowledged in the public #879 comparison, and #882 requests a joint-support audit. They should also be explicit here, alongside what the later checks establish. The population results below are saved development diagnostics reviewed on September 7: retained 2024 spines, 2025 model outputs, UK 2.94.0/Core 3.31.0 and Chronicle The historical paired SPI experiment improved lone-parent UC from 1.528m to 1.605m after calibration, but still missed its target by 27.92%. Its 39.81% total-loss improvement mainly came from interest, which supplied 91.5% of the net gain. The paired aggregate receipt therefore supports retaining valuable SPI improvements, without interpreting the overall loss reduction as resolution of UC composition. The subsequent checks separated mechanisms:
The target-contract comparison held all 371 target names/values, priors and family coefficients fixed: 1,500 updates, learning rate .02, seed 0, free total mass, weight/prior cap 10, loss cap 10, no sparsity penalty and the same best-iterate selection. For 111 compiled references, the guard requires each April–December observation exactly once: nine observed months, not a full calendar year. It reproduced the tested matrix and all solver inputs exactly. Objectives on changed measurement definitions are not directly comparable. The diagnostic weight vector respects those bounds; it does not pass every target/fit gate—the two empty rows still have −100% errors—or certify convergence or the population. Best-iterate selection fixes which result is returned within the requested budget; it cannot create missing support, and default epoch counts are unchanged. Two active payment cells remain entirely empty in that repaired matrix:
Both use the prefix These are zeros in the captured model support, not proof that those administrative payments are impossible. Earlier apparent childless-couple support came from a dependent being classified as a partner. Correcting the classification exposed the gap. Separately, DWP payment amounts include third-party payments and one-off advances in their issue month. Annual recurring model UC divided by twelve is not an exact cash-payment bridge. This PR fixes the finite £2,500 endpoint; the separately published £2,500.01+ category remains unbound. We should diagnose components/payment timing before generating recurring entitlement to fill those cells. The five-plus result is a different support warning. Previously, the matrix imposed The remaining work is therefore explicit:
|
faf146e to
61711ff
Compare
Partially addresses #882. Builds on merged #879 and #881, rebased onto official
mainat923cec2e.UC calibration should compare administrative claims with the corresponding benefit units and their own children. These changes prevent unrelated people in the same dwelling from contributing to a claim's child/disability categories, align GB coverage and finite payment-band bounds, and expose the approximations between annual model outputs and source statistics.
is_uc_claimantinput through graph storage, export and model loading.The UK extras require published PolicyEngine-UK 2.97.0, including the merged claimant-aware formulas in #1850, and the lock selects that release. Supplied FRS roles are authoritative: the model fallback cannot reconstruct a 17-year-old partner or distinguish every older dependent from the claimant's partner. Both export allow-lists permit the new input, and model-loading checks reject a supplied claimant column that the engine cannot consume. Reporter payment landing still selects one row by its established non-child/working-age ordering; this is separate from identifying the one or two members of the claim.
Best-iterate selection reaches the UK national free-mass production solve and the local rowwise default. Their returned weights can change. Epoch defaults, learning rates and weight bounds remain unchanged; the byte controls cover the paths excluded from this selection rule. These checks do not establish the effect on a rebuilt national population or certify new weights.
Current lock, seed, specification and UK gate-verifier mirrors follow their producers. Source comparisons isolate the solver implementation identity and claimant export allow-list addition; seed values, owner assignments, gate scopes and thresholds are unchanged. Historical execution receipts retain their actual runtime identities, including the frozen eFRS candidate's PolicyEngine-UK 2.89.0 provenance.
The rebase preserves #881's survey-year disability rules, survey-year grant/calibration-year DSA split and declared year-rule precedence alongside the claimant checks. Regeneration changes only three current identity artifacts: 13 source-manifest digest leaves, two H2 stage-contract hashes and the UK specification assertion. It does not relabel historical population evidence.
TCL measures remain positive-award and model-birth-year proxies for the administrative April snapshot. They do not reconstruct open/nil claims, exact birth dates or April claim history. The statistical definition includes exceptions; these proxies do not reconstruct administrative exception categories. Missing source crosses remain tracked in Chronicle #251; remaining consumer/support work stays in #882. The diagnostic results and remaining support limits identify the two active zero-support payment cells and the five-plus fit’s dependence on one original source household.
Validation:
mainat923cec2e; independent rebase/source/producer verification CLEAN.Latest commit
61711ff9is based on officialmainat923cec2e. GitHub CI PASSED all 23 jobs on this exact head and base. A full population rebuild and certification remain separate from code and synthetic checks.