Repository navigation
Fix household wealth deciles for tied values - #2583
Merged
Merged
Conversation
Contributor
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 84 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
|
juaristi22
marked this pull request as ready for review
October 23, 2026 00:26
vahid-ahmadi
left a comment
vahid-ahmadi
left a comment
Collaborator
There was a problem hiding this comment.
@juaristi22 thanks, first-round review at 44385d9. The fix is correct and does what #2579 asks. One should, on how the tie rule is documented and what it carries; the rest are notes.
What I checked
- New tests at head: 8 passed. With
household_wealth_decile.pyreset to main (d62d5f3b6), 6 of the 8 fail, including the reproducertest_zero_wealth_ties_fill_the_bottom_deciles, so the tests do pin the change. I also checked thenegative_wealthcase by hand (sorted cumulative person weights 1, 3, 4, 6, 10 out of 10 give 4, -1, 10, -1, 6 in row order). - Wealth YAML tests (
tests/policy/baseline/household/wealth,reforms/parametric/wealth_tax.yaml): 33 passed. Decile Python tests (test_household_income_decile.py,test_household_wealth_decile.py): 12 passed. - CI at head: all green. Changelog fragment present;
axiom: n/aandReproducer:lines fine. - On the certified data the aggregate-impact job uses (
enhanced_frs_2024_25.h5, policyengine-uk-data 1.58.1, sha256056e80fd…563784checked), 2027, person-weighted share of people by wealth decile:
| Decile | 1 | 2 | 3 | 4 to 10 |
|---|---|---|---|---|
| main | 0.0% | 20.0% | 10.0% | 10.0% each |
| this PR | 10.0% | 10.0% | 10.0% | 10.0% each |
15.0% of people live in households with exactly £0 of total_wealth, so on main they all land in decile 2 and decile 1 is empty, as #2579 reports. This PR fixes that.
Findings
-
should:
policyengine_uk/variables/household/wealth/household_wealth_decile.py:6,15-17. The tie rule is undocumented, and the split it makes is not neutral. The docstring still reads "Decile of wealth income (person-weighted)", while the siblinghousehold_income_deciledocuments its own (different) tie rule in prose. Here the 15% of people at £0 are split between deciles 1 and 2 by row order, which on this dataset ishousehold_idorder. That order carries information. Among £0-wealth households in 2027, the share owning outright is 4.1% in decile 1 and 9.0% in decile 2. Over 200 random orderings of the tied households, the decile 1 minus decile 2 gap had a standard deviation of 1.9 points, and none was as large as the 4.9 points from row order. So part of any decile 1 vs decile 2 contrast in the app is an artefact of file order, and it can move when the dataset is rebuilt and households are reordered. Please at least document the rule indocumentation: grosstotal_wealth, person-weighted, equal wealth split in household order, negative wealth set to -1. If you would rather make the split carry no information, one option is to break ties on a fixed hash ofhousehold_idinstead of row order; it stays deterministic and identical across baseline and reform. I checkednet_wealthas a secondary key: only 3.9% of people at £0 gross wealth have negative net wealth, so it would leave most ties unresolved. -
nit: the aggregate-impact comment on this PR says "pass", but its decile tables use
household_income_decile(policyengine_uk/utils/aggregate_impact/report_spec.yaml:51), so it does not test this change. The table above is the evidence instead. Worth saying in the PR body, since the body notes the private-data report was not run.
Context, no action for this PR
- ONS wealth deciles (Wealth and Assets Survey) are deciles of households ranked by total net wealth, including physical wealth. Ours are person-weighted on gross
total_wealth, which has no physical wealth. That narrower measure is why so many households sit at exactly £0 (the ONS bottom decile runs up to £16,500). Out of scope here. test_person_weighted_wealth_ranks[indivisible_weights]documents that large indivisible weights can still skip deciles. Fine as a stated limit.
Overlap and merge order: no file overlap with #2582 or any other open PR I found; both are mergeable, so they can merge in either order.
🤖 Generated with Claude Code
Collaborator
Author
|
Updated the variable documentation to specify gross wealth, person weights, the retained original-order tie rule and its order/rebuild sensitivity, negative outputs and indivisible-weight limits. I also clarified that the aggregate report groups by income decile, so its tables do not validate wealth-decile distributions. Only documentation and the existing changelog changed; executable ASTs and all other source bytes are unchanged. |
PavelMakarchuk
approved these changes
Oct 23, 2026
PavelMakarchuk
left a comment
PavelMakarchuk
left a comment
Collaborator
There was a problem hiding this comment.
Thanks María, this looks good. I approve.
I checked the new formula and the docs against the code. Ties now split in row order with person weights, outputs stay aligned, and negative wealth still becomes -1. The docs now cover everything Vahid asked for, and they match the code (total_wealth is gross: property values, corporate, pension and savings, no debt).
Tests: the new and income-decile tests pass (12), and the wealth YAML tests pass (33). With the old formula, 6 of the 8 new tests fail, so they pin the fix. Ruff passes, and CI is green.
Two small notes, neither blocking:
- Row order (
household_id) still decides how the £0 group splits between deciles 1 and 2, and a rebuild can move it. A fixed hash ofhousehold_idas the tie-breaker would keep it reproducible without following file order. Happy for that to be a follow-up issue. - A zero-weight household at the start of the order gets decile 0 (for example wealth [0, 0, 5], weights [0, 1, 1] → [0, 5, 10]). Main could already do this for a unique lowest value, but it can now happen inside a tie. A clip to at least 1 would avoid it.
I'll open a follow-up for both, stacked on this branch, so this PR can merge as it is.
vahid-ahmadi
approved these changes
Oct 23, 2026
vahid-ahmadi
left a comment
vahid-ahmadi
left a comment
Collaborator
There was a problem hiding this comment.
@juaristi22 thanks, second round at e29046a4c. The delta is documentation and changelog only (git diff 44385d9..e29046a), and I checked that the formula and tests are unchanged.
Earlier findings
- should, tie rule undocumented: resolved.
household_wealth_decile.py:6-14now states grosstotal_wealthbefore debt, thehousehold_weight * household_count_peopleweights, original household order within ties and its sensitivity to rebuilds, negative wealth ranked then set to -1, and the indivisible-weight limit. That is what I asked for. Breaking ties on a hash ofhousehold_idstays an option for later, not a blocker. - nit, aggregate-impact report does not test wealth deciles: resolved, the PR body now says so.
No new findings. CI is green at this head. Approving.
🤖 Generated with Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2579
Large groups with identical wealth can all fall into the same higher decile, leaving the bottom decile empty. Break ties in original household order while retaining person weights, output row alignment and the existing negative-wealth category.
Reproducer: policyengine_uk/tests/test_household_wealth_decile.py::test_zero_wealth_ties_fill_the_bottom_deciles
Validation: 12 focused Python checks and 24 wealth YAML cases passed;
make formatpassed. Full model CI and the private-data 2027 report were not run locally.Large indivisible record weights can still leave deciles empty. Equal-wealth ties follow the existing household order.
axiom: n/a: microsimulation wealth ranking; no legislative tax or benefit rule changes.
Documentation clarification: wealth deciles rank gross
total_wealthbefore debt deductions, withhousehold_weight * household_count_peopleweights and original household order within ties. The variable documentation now explains order/rebuild sensitivity, ranking negative wealth before assigning output-1, and indivisible-weight limits. The algorithm and tests are unchanged. Aggregate report decile tables usehousehold_income_decile; those tables do not validate the wealth-decile distribution.This documentation-only follow-up passed
make formatand lint. Static checks preserve all executable ASTs and every other tracked file byte. Model tests and the documentation build were not rerun for this prose-only change; earlier focused/YAML results remain dated original-head evidence.