diff --git a/changelog.d/1628-household-impact-diagnosis.changed.md b/changelog.d/1628-household-impact-diagnosis.changed.md new file mode 100644 index 0000000000..cc962b3f32 --- /dev/null +++ b/changelog.d/1628-household-impact-diagnosis.changed.md @@ -0,0 +1 @@ +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. diff --git a/docs/book/validation/policyengine-py-household-impact.md b/docs/book/validation/policyengine-py-household-impact.md index 1926c110cc..3cede5ab7e 100644 --- a/docs/book/validation/policyengine-py-household-impact.md +++ b/docs/book/validation/policyengine-py-household-impact.md @@ -2,26 +2,57 @@ ```{note} **Resolved.** [#1628](https://github.com/PolicyEngine/policyengine-uk/issues/1628) -reported that two `policyengine.py` household-impact integration tests -started asserting `0.0` for expected-positive tax and child-benefit -values after bumping the UK extra to `policyengine-uk==2.88.0`. This -page documents the diagnosis from the PolicyEngine UK side and the -guidance for the upstream `policyengine.py` repo. It is not a model -bug: the UK model produces the expected non-zero values when called -correctly. - -Re-checked in October 2026 on policyengine-uk 2.123.7: both scenarios -below still return £7,486 and £1,354.60. The upstream reproducer -(policyengine.py#280) was closed without merging. policyengine.py has -since replaced the `TestUKHouseholdImpact` tests with -`TestUKCalculateHousehold` in `tests/test_household_impact.py`, which -passes an explicit `year` to every case and pins policyengine-uk -2.125.1. #1628 can be closed once those tests are confirmed green. +reported that two `policyengine.py` household-impact integration tests, +which assert that income tax and child benefit are positive, got `0.0` +after +[policyengine.py#280](https://github.com/PolicyEngine/policyengine.py/pull/280) +moved the UK extra from `policyengine-uk==2.74.0` to `==2.88.0` and +raised the `policyengine_core` floor from 3.23.6 to 3.24.1. + +It was not a PolicyEngine UK bug. policyengine-core 3.24.0 to 3.24.3 +discarded inputs that a country package set before applying a reform +during simulation construction, the pattern `policyengine_uk.Simulation` +uses. +[policyengine-core#475](https://github.com/PolicyEngine/policyengine-core/pull/475) +fixed this in core 3.24.4, and +[policyengine.py#283](https://github.com/PolicyEngine/policyengine.py/pull/283) +raised policyengine.py's core floor to 3.25.0, both on 18 April 2026. + +On 22 October 2026 all eight UK cases in policyengine.py's +`TestUKCalculateHousehold` (`tests/test_household_impact.py`, which +replaced `TestUKHouseholdImpact` in the v4 launch) passed on its main CI +with policyengine-uk 2.132.6, so #1628 can be closed. ``` +## What went wrong + +policyengine-core's changelog for 3.24.4 describes the bug: its cache +invalidation after `apply_reform` "wiped every variable's +`_memory_storage._arrays`, which also wiped user-provided dataset inputs +loaded via `set_input`. Country-package subclasses calling `set_input` +during construction and then applying a structural reform (the +`policyengine-uk` pattern) silently lost their datasets." In the failing +tests, inputs such as age, `employment_income` and `would_claim_*` were +gone before any calculation ran, so tax and benefits came out as zero. + +Two other explanations were considered and do not fit the harness code +at policyengine.py#280's head (`6772ecc`): + +- **The dataset-required change** (policyengine-uk 2.76.0, where + `Simulation()` with no situation, no dataset and no + `POLICYENGINE_UK_DEFAULT_DATASET` raises a `ValueError`). The harness + passed an explicit dataset (`Microsimulation(dataset=...)` in + `src/policyengine/tax_benefit_models/uk/model.py`), so it never reached + that path. +- **The default period** (`default_input_period` and + `default_calculation_period`, 2025 in both 2.74.0 and 2.88.0). The + harness passed `period=` to every calculation, and the failing tests + built 2026 households. + ## Reproduction from the UK side -Running the two scenarios directly against `policyengine-uk` main: +Two comparable scenarios, run directly against `policyengine-uk` main for +2025 (the failing tests used 2026 and a one-adult, two-child family): ```python from policyengine_uk import Simulation @@ -53,67 +84,20 @@ print("child_benefit:", sim.calculate("child_benefit", YEAR).sum()) # -> 1354.60 ``` -Both return the expected non-zero values across the range 2022-2025 -(the income-tax figure is invariant because the personal allowance is -£12,570 throughout this window). So **the underlying model is healthy**. - -## What changed between 2.74 and 2.88 - -The change in that window that most plausibly explains the upstream -failure is the **dataset-required** invariant, added in 2.76.0 -("Stop defaulting to private dataset"): `Simulation()` with no -`situation=`, no explicit `dataset=` and no -`POLICYENGINE_UK_DEFAULT_DATASET` environment variable raises -`ValueError: Simulation() requires an explicit dataset when no -situation is provided`. If the `policyengine.py` harness was catching -that error and falling through to a degenerate code path (a -zero-filled `Microsimulation`), the tests would see zeros. - -The **default-period** contract is a second thing to rule out, although -it did not change in that window (both 2.74.0 and 2.88.0 default to -2025): - -```python -# policyengine_uk/simulation.py -class Simulation(CoreSimulation): - default_input_period: int = 2025 - default_calculation_period: int = 2025 -``` - -The failing tests built 2024 households. Any calculation that relied on -the default period would compute 2025, and any tax or benefit input -populated only for 2024 would be missing for that year. - -## Guidance for the upstream fix - -The `policyengine.py` household-impact harness should: - -1. Pass an **explicit `period`** to every `calculate(...)` call - matching the year encoded in the situation YAML, rather than relying - on the model default. -2. Build single-household cases with **`Simulation(situation=...)`** - rather than a dataset-backed simulation. They don't need a dataset - and shouldn't bring in the default-dataset env-var dependency. -3. **Assert against the year encoded in the test fixture**, not the - model's `default_input_period`. The fixture-encoded year is what the - test author intended. - -The two failing tests should produce ~£7,486 income tax for a -£50k-earning single adult and ~£1,354.60 of child benefit for a -couple-with-one-child benefit unit at 2025 rates. - -## Why this is documented in `policyengine-uk` rather than fixed there - -The actual fix lives in `policyengine.py`'s test harness and is -explicitly out of scope for this repo (sister-repo edits aren't -made from `policyengine-uk`). This page exists so the next reviewer -who sees `#1628` and is unfamiliar with the policyengine.py setup has -a one-stop diagnosis instead of having to re-derive it. +Re-checked on 23 October 2026 on policyengine-uk 2.148.2 (policyengine-core +3.33.2): both scenarios return £7,486 and £1,354.60. Both also return +non-zero values for every year from 2022 to 2025. The income-tax figure is +the same each year because the personal allowance (£12,570), the basic +rate limit (£37,700) and the 20% basic rate are unchanged from 2021-22; +child benefit follows the eldest-child rate (£1,133.60 in 2022, £1,248.00 +in 2023, £1,331.20 in 2024). ## References - Issue: [#1628](https://github.com/PolicyEngine/policyengine-uk/issues/1628). -- Reproducer: [`PolicyEngine/policyengine.py#280`](https://github.com/PolicyEngine/policyengine.py/pull/280). +- Where the failure showed: [`PolicyEngine/policyengine.py#280`](https://github.com/PolicyEngine/policyengine.py/pull/280) (closed without merging, superseded by #282). +- The fix: [`PolicyEngine/policyengine-core#475`](https://github.com/PolicyEngine/policyengine-core/pull/475) (core 3.24.4) and [`PolicyEngine/policyengine.py#283`](https://github.com/PolicyEngine/policyengine.py/pull/283) (core floor 3.25.0). - Default-period contract: [`policyengine_uk/simulation.py`](https://github.com/PolicyEngine/policyengine-uk/blob/main/policyengine_uk/simulation.py) (`default_input_period`, `default_calculation_period`). - Default-dataset env var contract: `POLICYENGINE_UK_DEFAULT_DATASET`, - defined in [`policyengine_uk/tax_benefit_system.py`](https://github.com/PolicyEngine/policyengine-uk/blob/main/policyengine_uk/tax_benefit_system.py). + defined in [`policyengine_uk/tax_benefit_system.py`](https://github.com/PolicyEngine/policyengine-uk/blob/main/policyengine_uk/tax_benefit_system.py) + and read in `policyengine_uk/simulation.py`.