Skip to content

Correct the policyengine.py household-impact diagnosis page (#1628) - #1729

Merged
vahid-ahmadi merged 6 commits into
mainfrom
vahid-ahmadi/issue-1628-household-impact-tests-diagnosis
Oct 24, 2026
Merged

Correct the policyengine.py household-impact diagnosis page (#1628)#1729
vahid-ahmadi merged 6 commits into
mainfrom
vahid-ahmadi/issue-1628-household-impact-tests-diagnosis

Conversation

@vahid-ahmadi

@vahid-ahmadi vahid-ahmadi commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Corrects the policyengine.py household-impact diagnosis page (docs/book/validation/policyengine-py-household-impact.md), written for #1628.

The page itself is already on main: it shipped in #2386 (merged 21 October 2026), and its original .added changelog entry was released in 2.143.0. This branch has merged main, dropped the released fragment and now carries only corrections to main's copy of the page.

What this PR corrects (from its changelog fragment):

Corrected the policyengine.py household-impact page (docs/book/validation/policyengine-py-household-impact.md): the zeros in #1628 came from a policyengine-core 3.24 bug fixed in 3.24.4, not from policyengine-uk; re-checked on 2.148.2.

The claim-by-claim review behind these corrections, with sources, is in the "Ready for hub review" comment on this PR.

The corrected page notes that the closing condition for #1628 is now met (policyengine.py's UK household-impact tests pass on its main CI); the issue itself is left for its author to close.

  • Changelog fragment: changelog.d/1628-household-impact-diagnosis.changed.md (changed).
  • Docs and changelog only; no code changes.

🤖 Generated with Claude Code

@vahid-ahmadi
vahid-ahmadi force-pushed the vahid-ahmadi/issue-1628-household-impact-tests-diagnosis branch from 86b3b80 to 5f34909 Compare October 8, 2026 14:51
vahid-ahmadi and others added 3 commits October 12, 2026 10:59
@vahid-ahmadi
Document policyengine.py household-impact test diagnosis (#1628)
6d8e866 
#1628 reported that two policyengine.py household-impact integration
tests started asserting 0.0 for expected-positive tax / child-benefit
values after bumping to policyengine-uk==2.88.0.

Reproducing the same scenarios directly against policyengine-uk shows
the model is healthy: a £50k single adult correctly returns £7,486
income tax, and a couple with one child correctly returns £1,354.60
child benefit at 2025 rates. The figures are stable across 2022-2025.

So the regression is upstream in policyengine.py's test harness, not
a model bug. Most likely cause: the harness depending on the model's
default_input_period (2025) vs the year encoded in the test fixture,
or being broken by the default-dataset env var requirement introduced
since the previous 2.74 pin.

Persist the diagnosis as a docs page so the next reviewer of #1628
gets a one-stop explanation. Guidance for the upstream fix:

- pass explicit period= to every calculate() call,
- use Microsimulation(situation=...) for single-household queries
  (no dataset needed),
- assert against the fixture-encoded year rather than the model's
  default.

Cross-links to mp30k-uk-switch-plan.md which documents the env-var
contract this issue collides with.
@vahid-ahmadi @claude
Refresh the page against current main and Microcosm UK; type the chan…
25383e8 
…gelog fragment

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vahid-ahmadi @claude
Correct the 2.74 to 2.88 diagnosis and note the upstream test rewrite
ad466f5 
The default period was already 2025 at 2.74.0, so it did not change in
that window; the dataset-required error arrived in 2.76.0. Note that
policyengine.py now tests with explicit years in TestUKCalculateHousehold,
and remove em dashes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vahid-ahmadi
vahid-ahmadi force-pushed the vahid-ahmadi/issue-1628-household-impact-tests-diagnosis branch from 5f34909 to ad466f5 Compare October 12, 2026 09:05
@vahid-ahmadi @claude
Format the code block for ruff
dd30612 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vahid-ahmadi vahid-ahmadi mentioned this pull request Oct 12, 2026
Merged
MaxGhenis and others added 2 commits October 23, 2026 02:04
@MaxGhenis @claude
Merge main into the policyengine.py household-impact page branch
2a7c07e 
Main already carries this page (#2386) and released its changelog line in 2.143.0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis @claude
Correct the household-impact zeros diagnosis to the core 3.24 bug
9340cb3 
The page reached main through #2386 and its changelog line shipped in
2.143.0, so this drops the released .added fragment and corrects main's
copy. The page blamed the 2.76.0 dataset-required change and the default
period; the harness at policyengine.py#280's head passed an explicit
dataset and period, and its tests built 2026 households. The actual cause,
per policyengine-core#475 and core's 3.24.4 changelog, was core 3.24.0 to
3.24.3 wiping set_input values on apply_reform; #280 raised the core floor
to 3.24.1 alongside the UK bump, and policyengine.py#283 moved it to
3.25.0.

- Cut the guidance and "why documented here" sections, which rested on
  the wrong diagnosis and cited a repo rule that does not exist.
- policyengine.py now pins policyengine-uk 2.132.6 and its eight
  TestUKCalculateHousehold cases pass on main CI (run 39822499276).
- Re-ran the page's code on 2.148.2: 7486.0 and 1354.60, non-zero for
  2022 to 2025.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis

MaxGhenis commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

UK hub (Claude build session, for Max)
Ready for hub review

Head: 9340cb37ad63af7b65d83cca8dbc0cf0494f93da

Read this first. The page is already on main, word for word, through #2386 (d50ee85a9, merged 2026-10-21). Its changelog line shipped in 2.143.0 (CHANGELOG.md:203). The page's diagnosis of #1628 is also wrong.

This branch now:

  • merges main (2a7c07efe);
  • deletes changelog.d/1628.added.md;
  • rewrites main's page around the actual cause;
  • adds changelog.d/1628-household-impact-diagnosis.changed.md.

Against main, the diff is the page and the fragment. Suggested title on landing: "Correct the policyengine.py household-impact diagnosis (#1628)".

Claims checked

Claim Verdict Source
#1628 tests "asserting 0.0" after the bump to 2.88.0 reworded: the tests assert > 0 and got 0.0 issue #1628 body
The cause was the 2.76.0 dataset-required change or the default period false the harness passed Microsimulation(dataset=input_data) and period=simulation.dataset.year (policyengine.py@6772ecc src/policyengine/tax_benefit_models/uk/model.py), and the failing tests built year=2026 households (tests/test_household_impact.py:41,59 at 6772ecc)
The actual cause policyengine-core 3.24.0 to 3.24.3 wiped set_input values on apply_reform; #280 raised the core floor to 3.24.1 alongside the UK bump policyengine-core#475 (merged 2026-04-18); core CHANGELOG.md ## [3.24.4] - 2026-04-18; policyengine.py#283 (core >=3.25.0, merged 2026-04-18); #280 pyproject.toml diff
2.76.0 added the dataset-required ValueError true (kept as a ruled-out hypothesis) commit f6e34cef4 is in 2.76.0 and not in 2.75.4; policyengine_uk/simulation.py:181-188
Both 2.74.0 and 2.88.0 default to 2025 true 3caccc8eb:policyengine_uk/simulation.py:80-81; 2.88.0:policyengine_uk/simulation.py:92-93
policyengine.py#280 closed without merging true "Superseded by #282"
TestUKCalculateHousehold replaced TestUKHouseholdImpact; explicit year true; replaced in v4 (28d8904a44, #298) policyengine.py main tests/test_household_impact.py:22
policyengine.py "pins policyengine-uk 2.125.1" outdated: 2.132.6 now policyengine.py main pyproject.toml:45,50,75
"#1628 can be closed once those tests are confirmed green" condition met: all 8 UK cases passed policyengine.py run 39822499276 ("Push to main", 8d76fff93, 2026-10-22), with policyengine-uk==2.132.6
Re-check "on 2.123.7" returns £7,486 and £1,354.60 re-run on 2.148.2 / core 3.33.2 (uv run --frozen in this branch's checkout): income_tax: 7486.0, child_benefit: 1354.5997 the page's code block, run unchanged
Non-zero across 2022-2025; income tax invariant true (re-run): income tax 7486.0 each year; child benefit 1133.6 / 1248.0 / 1331.2 / 1354.6 variant script; PA personal_allowance/amount.yaml:20-21, basic rate limit rates/uk.yaml:56-57
Default-period class excerpt true (section cut as no longer relevant) simulation.py:224-232
"Guidance for the upstream fix" (explicit period, situation=, fixture year) cut: rests on the wrong diagnosis; the harness already did the first and policyengine.py now builds Simulation(situation=...) —
"sister-repo edits aren't made from policyengine-uk" cut: unsupported; no such rule in CLAUDE.md, AGENTS.md or .github/CONTRIBUTING.md grep
Env var defined in tax_benefit_system.py true; added that simulation.py reads it tax_benefit_system.py:103; simulation.py:182

Corrections: 9.

Uncertain

  • I relied on core#475's own description and did not trace which step of policyengine_uk.Simulation.__init__ at 2.88.0 triggered the wipe.

Follow-up for the hub

#1628 is still open. Its closing condition is met, and it can be closed with a link to core#475 and policyengine.py#283. I haven't closed it.

Full ledger: ~/reviews/uk-hub/jobs/docs-c2/b/1729-ledger.md.

@MaxGhenis MaxGhenis mentioned this pull request Oct 23, 2026
Closed
3 of 5 tasks
@vahid-ahmadi vahid-ahmadi changed the title Document policyengine.py household-impact zero-return diagnosis (#1628) Correct the policyengine.py household-impact diagnosis page (#1628) Oct 23, 2026
@vahid-ahmadi
vahid-ahmadi added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit d0820b4 Oct 24, 2026
10 checks passed
@vahid-ahmadi
vahid-ahmadi deleted the vahid-ahmadi/issue-1628-household-impact-tests-diagnosis branch October 24, 2026 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vahid-ahmadi @MaxGhenis