Correct UC measurement contracts and calibration iterate selection - #883
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
vahid-ahmadi
left a comment
There was a problem hiding this comment.
Second pass at 61711ff9 (Claude Code, high effort; worktree checkout against main 923cec2e, the four response commits read against the round-one findings; 27 suites run: 1,045 passed, 35 skipped, 10 failed — the ten are all test_us_multispine_pool_tool.py failing on the missing policyengine-us extra identically on a clean main worktree, so environmental, not the PR; every round-one suite passed or skipped for a stated extras reason; ruff check and ruff format clean on all 45 changed Python files; both census tools, the coverage manifest and ci_test_groups --verify current; CI on the head 23 of 23 green. The UK extra is not installed here, so the requires_uk tests rest on CI.)
1. Fixed, verified. pins.json now carries 142dc3bf…, test_h1_kernel_parity passes with the bitwise comparison executing, and putting main's pin back reproduces the failure. direct.csv, graph.json and inputs.csv are unchanged against main; only the pin moved. Minor: the pin's platform label went py3.14 → py3.13, harmless for a bitwise kernel.
2. Fixed, verified. gates.json:621 and terminal_gates.py:210 both carry person.is_uc_claimant, the declared set equals the runtime tuple, uk_export_surface_gate now passes with the column and still refuses an unrelated one, and main's terminal_gates.py reproduces the round-one refusal text. Tests in test_uk_terminal_gates.py:273 and test_uk_graph.py; the gate-battery digests follow.
3. Addressed, with one receipt gap. The frozen optimizer is a vendored verbatim copy of solve.py from main commit 1f7d676f (fixtures/pre_best_iterate/optimizer_1f7d676f.py), and controls.json pins its module sha, the _optimize source sha, five helper source shas taken from the installed module, the gates.py sha and the prune tolerance; test_best_iterate_preserves_pre_change_excluded_paths runs oracle and current solver from separate seeds and asserts tobytes() equality on weights, the full loss trajectory and the L0 gate probabilities for the conserved-mass, L2 and L0 cases, plus the recorded Mac bytes on the authoring runtime — that branch executed here and passed. The tests live in the shared filter and run in the engine tier's other-shards process. The body and uc-calibration-best-iterate.fixed.md now say plainly that free-mass national and rowwise-local weights can change and that this establishes no national fit stability. Residual: iterate_selection lives only in calibrate()'s options; neither national_calibration.py:199-205 nor local_rowwise.py:1339-1342 copies it into the stage receipt, so a frozen manifest cannot show which iterate a free-mass solve returned. One field.
4. Addressed; the contract is stated, in docstrings. The three UK extras and the lock are at released 2.97.0. validate_uc_claimant_input (adapters/policyengine_uk.py:165-204, called from materialize and the direct-H5 resolver) refuses a definition that is not person/bool/year, a source column that is not complete Boolean, a column the loader did not retain as an input, and loaded values that differ from the supplied ones; six defect cases are tested and the direct-H5 refusal is tested both ways. The released-engine test covers the 24-year-old with a 17-year-old childless partner through final UC. Two notes: the validation returns silently when the column is absent from the frame (the production spine always emits it, so not blocking, but a frame without it gets the engine fallback with no signal); and the post-terminal-date 19-year-old dependent, the other divergent case from round one, is not in the released-engine test — every row there has the terminal-date flag true. The authoritative-roles contract is in uc_relationships.py:24-28, the adapter docstring and the PR body; nothing in the changelog or docs.
5. Verified. efrs_parity_known_gaps.json is unchanged against main at 2.89.0, and the new coverage-manifest test pins the frozen candidate's original engine provenance.
6. Rebase commits are regeneration. 20d29772 carries the actual fixes; 3cb98e02, dcf15048 and 61711ff9 move the approved lock digest, the US spec sha, the UK spec sha, the coverage-manifest shas and the parity fixture. The US surfaces move because microcosm.calibrate.solve is a direct kernel module whose source sha feeds every spec_sha256, so any solver edit ripples there by construction. Nothing out of scope.
7. Partial. The body says these checks do not establish the effect on a rebuilt population or certify new weights, and links the diagnostics comment; only that comment says the UC population miss is not closed and cites the −27.9% lone-parent residual. The changelog is silent on it. Since the diagnostics comment is the honest statement of where this leaves #882, one sentence of it belongs in the body proper.
Approving. Both blockers are fixed with the pre-fix failures reproduced, the solver change is now controlled byte-for-byte on the paths it claims not to touch and declared on the paths it does, and the claimant contract is enforced at both load paths. The four residuals — receipting iterate_selection, the silent-absence branch, the 19-year-old case, and the population-miss sentence — are all small and can follow.
…est-iterate oracle's gates attestation for the rebased tree (#355) Rebased onto main after #881 and #883. The country-bundle digests, the seed protocol and map, the loader golden vector, the US spec digest in the multispine pool-tool test, the regenerated coverage report and the H1 calibrate parity fixture are re-cut on the combined tree. Main's new pre-best-iterate oracle attests the gates module's bytes; #355's gates changes (per-record initial probabilities and the protected mask) move that digest legitimately, so the attestation is re-pinned and the oracle's byte-identity checks on the excluded paths still pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…est-iterate oracle's gates attestation for the rebased tree (#355) Rebased onto main after #881 and #883. The country-bundle digests, the seed protocol and map, the loader golden vector, the US spec digest in the multispine pool-tool test, the regenerated coverage report and the H1 calibrate parity fixture are re-cut on the combined tree. Main's new pre-best-iterate oracle attests the gates module's bytes; #355's gates changes (per-record initial probabilities and the protected mask) move that digest legitimately, so the attestation is re-pinned and the oracle's byte-identity checks on the excluded paths still pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.