Skip to content

Rank earnings quintiles over adults on potential earnings - #1829

Merged
vahid-ahmadi merged 2 commits into
mainfrom
fix-earnings-quintile-population
Aug 28, 2026
Merged

Rank earnings quintiles over adults on potential earnings#1829
vahid-ahmadi merged 2 commits into
mainfrom
fix-earnings-quintile-population

Conversation

@vahid-ahmadi

Copy link
Copy Markdown
Collaborator

Fixes #1827.

Stacked on #1828 (the hours_worked units fix), which adds the hourly_wage helper this uses. Base is set to that branch; it will retarget to main once #1828 merges. Review #1828 first.

The bug

calculate_earnings_quintile applies pd.qcut to raw employment_income across every person in the dataset, children included:

employment_income = sim.calculate("employment_income", year)
quintiles = pd.qcut(employment_income + rng.random(...), q=5, labels=[1,2,3,4,5], duplicates="drop")

More than half that population has zero earnings. On enhanced_frs_2024_25 (N = 113,617; 50.7% zero-earners; 20.5% under 18):

Q share zero-earners workers share children
1 100% 0 25.2%
2 100% 0 24.6%
3 53.5% 10,558 26.1%
4 0% 22,723 25.5%
5 0% 22,724 0.9%

Two consequences

Elasticities are skewed. calculate_participation_elasticities indexes OBR Table A1 by this quintile, and the table rises steeply as the quintile falls — lone_parent_3_5 runs [1.554, 1.243, 0.932, 0.621, 0.155], a 10× spread. Every potential entrant sat at the steep end.

Most non-workers could never enter employment. impute_wages_for_nonworkers drew donors from the same elasticity groups, requiring (elasticities == v) & working_mask to be non-empty. The lowest quintiles had no workers, so imputation silently returned zero:

Q non-working adults share with £0 imputed wage
1 16,989 100%
2 17,122 56.1%

and apply_participation_responses gates entry on it:

if np.random.random() < entry_probability and imputed_wages[i] > 0:

So roughly 24,000 of 43,000 non-working adults were structurally incapable of entering work under any reform. No error, just no entrants.

Against the existing docstring

For workers, uses actual earnings. For non-workers, uses imputed potential earnings.

That is what the function was meant to do. It could not, while the imputation depended on the quintile — the dependency was circular. hours_for_new_entrants was accepted and never used.

Changes

  • Wage donors grouped by sex and age band instead of elasticity group. This breaks the circular dependency (the arrow now runs imputation → quintile, one way) and guarantees every non-worker a donor pool. Uses the weighted median rather than the mean, since a small share of workers have implausibly low implied hourly wages.
  • Quintiles ranked over adults on potential earnings — actual for workers, imputed for non-workers.
  • Assignment by weighted rank, not value cutoffs. Every non-worker in a donor group shares one imputed value, so potential earnings carry large point masses; cutting on values drops a whole mass into one quintile and can leave others empty (I hit exactly this in a first attempt). Ranking on the midpoint of each person's weight splits a tied mass across the boundary.
  • random_seed is retained in the signature for compatibility and documented as unused.

Effect on enhanced_frs_2024_25 at 2027

before after
adults per quintile uneven, Q1/Q2 no earners 20.0% each
non-working adults with £0 imputed wage 100% of Q1 0.0%
mean imputed entrant earnings £212 (and £0 for Q1) £18,004 (£18.42/hour)
mean participation elasticity, non-workers 0.251 0.237

Not fixed here

I have not asserted what quintile definition the OBR intended — I could not extract text from the OBR note. The case rests on the implementation's own terms: two quintiles that are entirely non-earners, contain no observed wage, and block entry for everyone in them cannot be a sound input to a table indexed by earnings rank.

The ranking is now weighted; it was previously unweighted over the microdata array.

Testing

policyengine_uk/tests/test_participation_earnings_quintile.py .......       [100%]
7 passed

Plus the 4 from #1828. Existing test_labour_supply_response_formulas.py and test_behavioral_responses.py still pass (17 passed, 11 skipped without a dataset token). ruff format clean.

@juaristi22

Copy link
Copy Markdown
Collaborator

Adversarial review — policyengine-uk PR 1829

Verdict: REQUEST CHANGES.

  1. CRITICAL — participation.py:434-439: tied non-workers are assigned by input row order. A permutation of the PR fixture changed 28 adult quintiles. This makes elasticities and stochastic responses depend on serialization order.
  2. CRITICAL — participation.py:383-402,414-439: the central distribution is not supported by the cited OBR note. The note identifies UK earnings quintiles but does not say to rank workers and non-workers together on imputed potential earnings. Prefer weighted thresholds estimated from observed workers and map imputed wages to those thresholds.
  3. SHOULD ADDRESS — participation.py:263-285: zero-weight donors bypass the fallback. A direct reproducer leaves a non-worker at £0 imputed earnings.

Validation: 17 relevant tests passed across focused and broader sets, 11 credential-dependent tests skipped, Ruff passed. Both adversarial failures reproduce despite the passing tests. No check runs were attached to the head.

vahid-ahmadi added a commit that referenced this pull request Aug 28, 2026
Addresses review on #1829.

Ranking a pooled distribution of workers' actual and non-workers' imputed
earnings split the tied mass of non-workers across a quintile boundary, and
which member of the mass fell on which side depended on the order of rows in
the dataset. A permutation of the test fixture moved 23 of 80 adults, all of
them non-workers. Elasticities and stochastic responses should not vary with
serialisation.

Thresholds are now the weighted quintile boundaries of the observed earnings
distribution of working adults, and everyone is placed against them by value:
workers on actual earnings, non-workers on imputed potential earnings. A tied
mass lands wholly in the quintile its value belongs to, so the result is
independent of row order — verified over permutations in the tests.

This also drops the claim that the OBR note prescribes ranking workers and
non-workers together, which it does not say. Thresholds estimated from
observed earnings are a weaker and better-supported assumption.

Quintiles are consequently not equal shares of the adult population: workers
divide roughly 20% each, while non-workers concentrate where their imputed
wage falls. That is intended — these are quintiles of the earnings
distribution, which is what Table A1 is indexed on.

weighted_median now falls back to the unweighted median for a group carrying
no weight, rather than returning zero. A zero there silently bars entry into
employment, and the previous `if donors.any()` guard did not catch it: a
non-empty but weightless donor group returned zero and bypassed the fallback.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

All three reproduced. Pushed fixes for each; the first two changed the approach rather than patching it.

1. Tie-order dependence — confirmed and fixed

Reproduced directly: permuting the fixture moved 23 of 80 adult quintiles, all of them non-workers tied on a shared imputed value. np.argsort(kind="stable") preserves input order for ties, so serialisation decided who fell on which side of a boundary.

Fixed by taking your suggestion in point 2 rather than by adding a tiebreak, since that resolves both issues at once. Verified over 20 permutations: 0 changes, and there is now a test asserting it.

2. The pooled ranking is not supported by the OBR note — accepted

You are right, and I should not have leaned on the note for that. I had already dropped the claim about what the OBR intended from #1827 for the same reason, but the implementation still embodied it.

Thresholds are now the weighted quintile boundaries of the observed earnings distribution of working adults, with everyone placed against them by value — workers on actual earnings, non-workers on imputed potential earnings. That is the weaker, better-supported assumption you proposed, and it is what makes the result order-independent: a tied mass lands wholly in the quintile its value belongs to.

The visible consequence, which I have documented in the docstring: quintiles are no longer equal shares of the adult population. On enhanced_frs_2024_25 at 2027, workers divide 19.8 / 20.1 / 20.0 / 20.1 / 20.0 across the five, while non-workers concentrate in Q2, which ends up holding 54% of adults. That is inherent to placement rather than ranking, and it is the correct behaviour for quintiles of the earnings distribution — but it is lumpy because the imputation assigns one median wage per sex and age band. A finer imputation would spread it. Worth knowing before anyone reads the quintile shares as population shares.

3. Zero-weight donors — confirmed and fixed

Your reproducer was right and my if donors.any() guard did not catch it: a non-empty but weightless donor group returned a weighted median of zero and used it, bypassing the fallback entirely. weighted_median now falls back to the unweighted median when a group carries no weight, and only a genuinely empty group returns zero. Test added with your case.

Where things stand on real data

before after
non-working adults with £0 imputed wage 100% of Q1 0.0%
every quintile contains earners no yes
quintile assignment stable under row permutation no yes
mean participation elasticity, non-working adults 0.251 0.239

13 tests in the two new files, 19 passed across the broader labour supply set, ruff format clean.

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial re-review — request changes

The recent changes resolve the two main issues from my first review: fixed thresholds remove the row-order/tie dependence, and deriving those thresholds from observed worker earnings is the right distributional construction.

BLOCKING — zero-weight records still affect modeled outcomes (participation.py:278-293, 385-400).

weighted_median now falls back to the unweighted median when every donor has zero weight. That prevents impute_wages_for_nonworkers from reaching its documented overall-median fallback, so observations representing no population can dictate the imputed wage. In a minimal case, a zero-weight same-cell donor earning £1,000,000 and a positive-weight overall donor earning £40,000 produce £501,333 of imputed annual earnings for the nonworker.

weighted_quantiles has the same root problem: zero-weight observations remain in the interpolation array. Positive-weight values [10, 20, 30, 40, 50] give thresholds [15, 25, 35, 45]; inserting a zero-weight value of 29 changes them to [15, 29, 35, 45]. A zero-population row must not move a population-weighted boundary.

Please filter to finite, strictly positive weights for both donor medians and worker thresholds. If a demographic cell has no positive-weight donor, fall back to the overall positive-weight worker median. Add invariance tests showing that adding, removing, or changing zero-weight rows cannot affect imputation or quintiles.

The integration-style regression discussed on #1828 that executes the new-entrant hours_worked writeback is also not yet present at this head; per the thread there, #1829 can carry it.

Validation: 19 focused tests passed, 11 credential-dependent tests skipped; Ruff, formatting, and git diff --check passed. The two adversarial weight-invariance cases above exposed the remaining issue.

@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

One further thing found while writing the integration test you asked for, which I have not fixed here and want on the record before this merges.

The entry path crashes. apply_participation_responses writes a float64 imputed wage into a float32 series, and pandas 3 raises rather than upcasting:

TypeError: Invalid value '[20000. 30000. 45000. 70000. 15435.79 15435.79 15435.79]' for dtype 'float32'

It reproduces on a small situation-based simulation as soon as anyone actually enters work. There are at least two sites — calculate_gain_to_work around line 359, and the new_employment_income / new_hours_worked writebacks.

This is latent on main: imputed wages there are zero for most non-workers, so imputed_wages[i] > 0 is false and the entry branch never executes. This PR is what makes entry possible for the first time, so it is also what exposes the crash. Nothing in production calls it today — apply_labour_supply_responses has the participation model commented out — but the code cannot be enabled as it stands.

I stopped short of fixing it because it is a distinct defect from the two this PR addresses, and bundling a dtype fix into a change about quintiles and donor pools would make both harder to review. Happy to either add it here or file it separately — say which you prefer.

It does mean the integration test you asked for on #1828 cannot be written until this is fixed, since forcing entry is exactly what triggers the crash. That is the honest reason it is not in this PR yet.

Base automatically changed from fix-participation-hours-units to main August 28, 2026 11:58
vahid-ahmadi added a commit that referenced this pull request Aug 28, 2026
Addresses review on #1829.

Ranking a pooled distribution of workers' actual and non-workers' imputed
earnings split the tied mass of non-workers across a quintile boundary, and
which member of the mass fell on which side depended on the order of rows in
the dataset. A permutation of the test fixture moved 23 of 80 adults, all of
them non-workers. Elasticities and stochastic responses should not vary with
serialisation.

Thresholds are now the weighted quintile boundaries of the observed earnings
distribution of working adults, and everyone is placed against them by value:
workers on actual earnings, non-workers on imputed potential earnings. A tied
mass lands wholly in the quintile its value belongs to, so the result is
independent of row order — verified over permutations in the tests.

This also drops the claim that the OBR note prescribes ranking workers and
non-workers together, which it does not say. Thresholds estimated from
observed earnings are a weaker and better-supported assumption.

Quintiles are consequently not equal shares of the adult population: workers
divide roughly 20% each, while non-workers concentrate where their imputed
wage falls. That is intended — these are quintiles of the earnings
distribution, which is what Table A1 is indexed on.

weighted_median now falls back to the unweighted median for a group carrying
no weight, rather than returning zero. A zero there silently bars entry into
employment, and the previous `if donors.any()` guard did not catch it: a
non-empty but weightless donor group returned zero and bypassed the fallback.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
@vahid-ahmadi
vahid-ahmadi force-pushed the fix-earnings-quintile-population branch from e80d3cf to 5f05bee Compare August 28, 2026 12:00

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

Source Documents

  • Official note: OBR, NICs cut impact on labour supply (Table A1)
  • Scope: Full merge-base review, carrying forward prior findings because the earlier report did not record a reviewed SHA
  • Reviewed head SHA: 5f05bee62c702fe1d5f56d87eae4d67352af1088
  • Mode: full

Critical (Must Fix)

  1. [C3] Towncrier still ignores the PR's changelog fragment. changelog.d/1827.md has no configured type; towncrier build --draft renders “No significant changes.” Rename it with a valid type suffix or place it in a configured type directory. — changelog.d/1827.md:1

Should Address

  1. [A2] Adults in a population with no workers are assigned to quintile 5. With no worker observations, weighted_quantiles returns four zero thresholds and searchsorted(..., side="right") assigns every non-working adult to the top quintile. A focused reproducer returned [5, 5]; add an explicit fallback and boundary test. — policyengine_uk/dynamics/participation.py:458

Resolved Prior Findings

  • [C1 RESOLVED] Fixed worker thresholds remove row-order-dependent splitting of tied imputed earnings; five shuffle tests pass.
  • [C2 RESOLVED] Workers define the earnings distribution; non-workers are placed against fixed thresholds rather than pooled into the ranking.
  • [A1 RESOLVED] Zero-weight donor groups now fall back to an unweighted median and the overall donor fallback.

Validation Summary

  • Focused tests: 9 passed; row-order reproducer fixed.
  • New no-worker reproducer: [5, 5], confirming A2.
  • Ruff: passed on changed Python files.
  • CI: 8/8 checks passed on the exact head.

Review Severity: REQUEST_CHANGES

Next Steps

Fix the Towncrier path and define/test the no-worker fallback.

@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

Both fixed.

A2 — a population with no workers put every adult in the top quintile. Reproduced. With no observed earnings there is no distribution: weighted_quantiles returns four zeros and searchsorted(..., side="right") sends everyone to Q5 — the end of the OBR table with the smallest elasticities, which is the opposite of what a population of non-workers implies.

My first attempt at a fallback did not work, and the reason is worth recording: I fell back to thresholds from potential earnings, but with no workers there are no wage donors either, so imputed wages are zero and the thresholds stay degenerate. The fix now keys on the degeneracy itself — if the thresholds carry no dispersion, adults are spread evenly by weighted rank rather than asserting a quintile. Zero total weight falls back to the middle quintile.

Real data is unaffected, since workers exist and the thresholds have dispersion: quintile shares of workers remain 19.8 / 20.1 / 20.0 / 20.1 / 20.0.

C3 — changelog. Renamed to 1827.fixed.md.

Thanks for confirming C1, C2 and A1 as resolved, and for the no-worker reproducer — it is the kind of degenerate input I would not have constructed.

Still outstanding on this PR and not fixed here: apply_participation_responses raises a dtype error the moment anyone actually enters work, because a float64 imputed wage is written into a float32 series. It is latent on main (imputed wages are ~0 there, so entry never fires) and only reachable because of this PR. I have a second review running on exactly that and will either fix it here or file it separately once I know every site that needs it.

vahid-ahmadi added a commit that referenced this pull request Aug 28, 2026
Addresses review on #1829.

Ranking a pooled distribution of workers' actual and non-workers' imputed
earnings split the tied mass of non-workers across a quintile boundary, and
which member of the mass fell on which side depended on the order of rows in
the dataset. A permutation of the test fixture moved 23 of 80 adults, all of
them non-workers. Elasticities and stochastic responses should not vary with
serialisation.

Thresholds are now the weighted quintile boundaries of the observed earnings
distribution of working adults, and everyone is placed against them by value:
workers on actual earnings, non-workers on imputed potential earnings. A tied
mass lands wholly in the quintile its value belongs to, so the result is
independent of row order — verified over permutations in the tests.

This also drops the claim that the OBR note prescribes ranking workers and
non-workers together, which it does not say. Thresholds estimated from
observed earnings are a weaker and better-supported assumption.

Quintiles are consequently not equal shares of the adult population: workers
divide roughly 20% each, while non-workers concentrate where their imputed
wage falls. That is intended — these are quintiles of the earnings
distribution, which is what Table A1 is indexed on.

weighted_median now falls back to the unweighted median for a group carrying
no weight, rather than returning zero. A zero there silently bars entry into
employment, and the previous `if donors.any()` guard did not catch it: a
non-empty but weightless donor group returned zero and bypassed the fallback.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
@vahid-ahmadi
vahid-ahmadi force-pushed the fix-earnings-quintile-population branch from 5f05bee to beec09e Compare August 28, 2026 12:43
vahid-ahmadi added a commit that referenced this pull request Aug 28, 2026
Addresses review on #1829.

Ranking a pooled distribution of workers' actual and non-workers' imputed
earnings split the tied mass of non-workers across a quintile boundary, and
which member of the mass fell on which side depended on the order of rows in
the dataset. A permutation of the test fixture moved 23 of 80 adults, all of
them non-workers. Elasticities and stochastic responses should not vary with
serialisation.

Thresholds are now the weighted quintile boundaries of the observed earnings
distribution of working adults, and everyone is placed against them by value:
workers on actual earnings, non-workers on imputed potential earnings. A tied
mass lands wholly in the quintile its value belongs to, so the result is
independent of row order — verified over permutations in the tests.

This also drops the claim that the OBR note prescribes ranking workers and
non-workers together, which it does not say. Thresholds estimated from
observed earnings are a weaker and better-supported assumption.

Quintiles are consequently not equal shares of the adult population: workers
divide roughly 20% each, while non-workers concentrate where their imputed
wage falls. That is intended — these are quintiles of the earnings
distribution, which is what Table A1 is indexed on.

weighted_median now falls back to the unweighted median for a group carrying
no weight, rather than returning zero. A zero there silently bars entry into
employment, and the previous `if donors.any()` guard did not catch it: a
non-empty but weightless donor group returned zero and bypassed the fallback.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

A second adversarial pass found three more, two of them serious. All fixed, and two of my own claims were wrong.

The dtype crash is unconditional, not entry-conditional — and I had the wrong site. It fires on the first calculate_gain_to_work call, at the in-work assignment, long before any entry decision. So apply_participation_responses never reached an entry at all, with any reform including a null one. My guess that the new_hours_worked writeback was implicated was wrong: 18.8 * 52 = 977.6 is float32-representable and that assignment succeeds. Exactly two sites needed casting, both imputed-wage assignments. Fixed, and calculate_gain_to_work now runs on the real dataset — 113,617 rows, mean gain to work £9,848.

My explanation of why this is latent on main was also wrong. I said imputed wages there are ~0 so entry never fires. They are not — mean £224 — and the assignment survives because main takes np.mean over a float32 array and gets a float32 mean, so every product is float32-exact. My weighted_median returns a Python float and multiplies by a float64 18.8, which is what breaks it. This PR converts a coincidence into a hard failure rather than exposing a dormant bug.

Non-worker quintiles carried almost no information, and it was a unit mismatch. Imputed earnings assume 18.8 hours a week while the thresholds come from observed earnings at whatever hours workers actually do — mostly full time. That pushed every non-worker down by roughly the hours ratio: all 12 distinct imputed values fell inside a single threshold interval, so 98.3% landed in Q2 and Q3–Q5 were unreachable for any reform on any dataset.

Non-workers are now placed on a full-time-equivalent basis, with the entrant's actual earnings left at the assumed hours. On the real data at 2027:

before after
quintiles reachable by non-workers 1 of 5 in practice 3 of 5
share in a single quintile 98.3% (Q2) 62.2% (Q3)
mean elasticity, non-working adults 0.214 0.163

Workers stay at 19.8 / 20.1 / 20.0 / 20.1 / 20.0. Three of five rather than five is still not ideal, and the cause is the coarse donor grouping — only 12 distinct imputed values, one per sex and age band. That is your finding 5 and I have not changed the banding here.

weighted_quantiles had no degenerate-weight fallback while weighted_median did. All-zero weights returned all-zero thresholds, placing every positive value in the top quintile — the lowest-elasticity end, the opposite of what a weightless population implies. Now falls back to unweighted quantiles, with a test.

Also fixed from the earlier pass: the no-worker case (my first attempt at that fallback did not work, because with no workers there are no donors either, so potential earnings were degenerate too) and the changelog type suffix.

Not addressed, and I think correctly out of scope here: the coarse age bands, weighted_median's NaN handling, the missing period arguments in calculate_participation_elasticities, children setting the pd.cut bin edges, and the global np.random.seed. The last four are pre-existing. Happy to file them.

You are right that the green suite was not evidence the PR worked end to end — every test was FakeSimulation-based and nothing reached apply_participation_responses, which is commented out of its coordinator. A single integration test would have caught the crash. That is worth adding once the module is enabled.

@vahid-ahmadi
vahid-ahmadi force-pushed the fix-earnings-quintile-population branch from beec09e to 9218c11 Compare August 28, 2026 12:47
calculate_earnings_quintile applied pd.qcut to raw employment_income across
every person in the dataset, children included. More than half that
population has no earnings, so the bottom two quintiles were entirely
non-earners and roughly a quarter of each lower quintile were children.

Two consequences. The OBR Table A1 elasticities are indexed by this quintile
and rise steeply as it falls — lone_parent_3_5 runs from 1.554 at Q1 to
0.155 at Q5 — so every potential entrant sat at the steep end. And because
impute_wages_for_nonworkers drew donors from the same elasticity groups, the
lowest quintiles had no employed donors, so their non-workers were imputed a
wage of zero. apply_participation_responses gates entry on
imputed_wages[i] > 0, so those people could never enter employment under any
reform — silently, with no error.

The function's docstring already described the intended behaviour ("For
workers, uses actual earnings. For non-workers, uses imputed potential
earnings"); it was not implemented, and could not be while the imputation
depended on the quintile.

Wage donors are now grouped by sex and age band, which breaks that circular
dependency, and quintiles are ranked over adults on potential earnings.
Assignment is by weighted rank rather than value cutoffs because every
non-worker in a donor group shares one imputed value, and cutting on values
drops a whole tied mass into a single quintile.

On enhanced_frs_2024_25 at 2027: quintiles are 20.0% of adults each, no
non-working adult is left with a zero imputed wage (previously 100% of Q1),
and imputed entrant earnings average £18,004, an implied £18.42 an hour.

Fixes #1827

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
vahid-ahmadi added a commit that referenced this pull request Aug 28, 2026
Addresses review on #1829.

Ranking a pooled distribution of workers' actual and non-workers' imputed
earnings split the tied mass of non-workers across a quintile boundary, and
which member of the mass fell on which side depended on the order of rows in
the dataset. A permutation of the test fixture moved 23 of 80 adults, all of
them non-workers. Elasticities and stochastic responses should not vary with
serialisation.

Thresholds are now the weighted quintile boundaries of the observed earnings
distribution of working adults, and everyone is placed against them by value:
workers on actual earnings, non-workers on imputed potential earnings. A tied
mass lands wholly in the quintile its value belongs to, so the result is
independent of row order — verified over permutations in the tests.

This also drops the claim that the OBR note prescribes ranking workers and
non-workers together, which it does not say. Thresholds estimated from
observed earnings are a weaker and better-supported assumption.

Quintiles are consequently not equal shares of the adult population: workers
divide roughly 20% each, while non-workers concentrate where their imputed
wage falls. That is intended — these are quintiles of the earnings
distribution, which is what Table A1 is indexed on.

weighted_median now falls back to the unweighted median for a group carrying
no weight, rather than returning zero. A zero there silently bars entry into
employment, and the previous `if donors.any()` guard did not catch it: a
non-empty but weightless donor group returned zero and bypassed the fallback.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
@vahid-ahmadi
vahid-ahmadi force-pushed the fix-earnings-quintile-population branch from 9218c11 to 0b60e09 Compare August 28, 2026 13:23
vahid-ahmadi added a commit that referenced this pull request Aug 28, 2026
Addresses review on #1829.

Ranking a pooled distribution of workers' actual and non-workers' imputed
earnings split the tied mass of non-workers across a quintile boundary, and
which member of the mass fell on which side depended on the order of rows in
the dataset. A permutation of the test fixture moved 23 of 80 adults, all of
them non-workers. Elasticities and stochastic responses should not vary with
serialisation.

Thresholds are now the weighted quintile boundaries of the observed earnings
distribution of working adults, and everyone is placed against them by value:
workers on actual earnings, non-workers on imputed potential earnings. A tied
mass lands wholly in the quintile its value belongs to, so the result is
independent of row order — verified over permutations in the tests.

This also drops the claim that the OBR note prescribes ranking workers and
non-workers together, which it does not say. Thresholds estimated from
observed earnings are a weaker and better-supported assumption.

Quintiles are consequently not equal shares of the adult population: workers
divide roughly 20% each, while non-workers concentrate where their imputed
wage falls. That is intended — these are quintiles of the earnings
distribution, which is what Table A1 is indexed on.

weighted_median now falls back to the unweighted median for a group carrying
no weight, rather than returning zero. A zero there silently bars entry into
employment, and the previous `if donors.any()` guard did not catch it: a
non-empty but weightless donor group returned zero and bypassed the fallback.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
@vahid-ahmadi
vahid-ahmadi force-pushed the fix-earnings-quintile-population branch from 0b60e09 to b6f2088 Compare August 28, 2026 13:26

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

Source Documents

Critical (Must Fix)

  1. [C1] The new degenerate-distribution fallback is still row-order dependent. When all four thresholds are equal, lines 510–517 sort tied potential earnings stably and assign quintiles by cumulative row rank. For ten identical non-working adults, the function returned [1,1,2,2,3,3,4,4,5,5]; after shuffling the input and restoring the original identities it returned [4,2,2,5,1,1,3,4,5,3]. Thus identical records receive different elasticities solely because of serialization order, contradicting the function's documented invariant. This path covers all-non-worker and zero-dispersion simulations. Preserve ties as a block or use a deterministic characteristic-based fallback, and add a permutation test that enters this branch. — policyengine_uk/dynamics/participation.py:503

Should Address

  1. [A1] Full-time-equivalent scaling is a material, unsupported extrapolation from the cited method. The OBR says entrants are assumed to work 18.8 hours weekly and labels Table A1 by position in the UK earnings distribution; it does not say to multiply their imputed in-work income to 37.5-hour FTE for elasticity assignment. The underlying IFS method is even less direct: it applies the calibration to existing workers and says non-workers are not allocated elasticities, with their potential earnings only implicit in the group judgement. On the focused fixture, the new factor moves every entrant from Q1 at £25,838 imputed part-time income to Q3, cutting the assigned Table A1 elasticity by about 40% for each demographic row. The latest code now acknowledges that real-data entrants still occupy only Q3/Q4, so the table remains near-degenerate; this makes a calibration or sensitivity result more important, not less. Either support 37.5/FTE placement with evidence or retain the 18.8-hour potential income and document a sensitivity range. — policyengine_uk/dynamics/participation.py:477

Resolved Prior Findings

  • [C2 RESOLVED] Worker-derived thresholds avoid splitting tied non-worker values in the ordinary, non-degenerate path.
  • [A2 RESOLVED] A no-worker population is no longer forced wholly into quintile 5, although its replacement fallback introduces C1.
  • [C3 RESOLVED] changelog.d/1827.fixed.md is now consumed by Towncrier.
  • [A3 RESOLVED] Zero/non-finite total weights fall back to unweighted quantiles; pandas 3 dtype assignments are handled explicitly.
  • [A4 RESOLVED] The latest commit filters non-finite value/weight pairs and rejects mismatched shapes in weighted_median.

Validation Summary

  • Focused tests: 13 passed.
  • Adversarial permutation reproducer: failed invariance on the exact head (invariant False).
  • FTE sensitivity fixture: non-workers move from Q1 to Q3.
  • Towncrier: fragment renders under Fixed.
  • Exact-head GitHub CI at final check: 7 passed, Test in progress; GitHub reports the PR mergeable.

Review Severity: REQUEST_CHANGES

Next Steps

Make the equal-threshold path tie- and order-invariant, then justify or sensitivity-test the FTE placement assumption.

vahid-ahmadi added a commit that referenced this pull request Aug 28, 2026
Addresses review on #1829.

Ranking a pooled distribution of workers' actual and non-workers' imputed
earnings split the tied mass of non-workers across a quintile boundary, and
which member of the mass fell on which side depended on the order of rows in
the dataset. A permutation of the test fixture moved 23 of 80 adults, all of
them non-workers. Elasticities and stochastic responses should not vary with
serialisation.

Thresholds are now the weighted quintile boundaries of the observed earnings
distribution of working adults, and everyone is placed against them by value:
workers on actual earnings, non-workers on imputed potential earnings. A tied
mass lands wholly in the quintile its value belongs to, so the result is
independent of row order — verified over permutations in the tests.

This also drops the claim that the OBR note prescribes ranking workers and
non-workers together, which it does not say. Thresholds estimated from
observed earnings are a weaker and better-supported assumption.

Quintiles are consequently not equal shares of the adult population: workers
divide roughly 20% each, while non-workers concentrate where their imputed
wage falls. That is intended — these are quintiles of the earnings
distribution, which is what Table A1 is indexed on.

weighted_median now falls back to the unweighted median for a group carrying
no weight, rather than returning zero. A zero there silently bars entry into
employment, and the previous `if donors.any()` guard did not catch it: a
non-empty but weightless donor group returned zero and bypassed the fallback.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
@vahid-ahmadi
vahid-ahmadi force-pushed the fix-earnings-quintile-population branch from b6f2088 to 5ede79a Compare August 28, 2026 13:36
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

@juaristi22 C1 confirmed and fixed; A1 documented rather than changed, with the bound quantified.

C1 — the degenerate fallback was still order-dependent. Reproduced exactly as you describe: ten identical non-working adults returned [1,1,2,2,3,3,4,4,5,5], and permuting the rows and restoring identities gave a different assignment. The cause is that I spread each row across its own slice of cumulative weight, and np.argsort(kind="stable") breaks ties in input order — so serialisation decided the split, which is the invariant this function is supposed to hold.

The fix blocks on the value rather than the row: each distinct potential-earnings value is placed at the midpoint of its own block of weight, so identical records are placed identically by construction. Ten identical adults now all get quintile 3 — the middle of the table, which is the honest answer when there is no dispersion to read — while genuinely distinct values are still spread rather than collapsed.

Two tests added, both driving the real calculate_earnings_quintile through the equal-threshold branch rather than reimplementing it: one asserting identical records share a quintile, one asserting permutation invariance. Both fail on the previous code with your [1,1,2,2,3,3,4,4,5,5], and pass now.

A1 — FTE placement. I agree this is an extrapolation the sources do not make, and I am not going to dress it up as one they do. Rather than assert it, the module now records it as an assumption with the direction and size of the bound: unscaled 18.8-hour placement moves entrants to quintile 1 and roughly doubles the elasticity each demographic row draws, and callers wanting that bound can pass hours_for_new_entrants=FULL_TIME_WEEKLY_HOURS to disable the scaling. Neither end is validated against an observed entry rate, which is the real gap — filed as part of #1836.

One correction to my own earlier comment while re-running on enhanced_frs_2024_25: I had written that non-workers occupy quintiles 3 and 4 only. With the current head they span 2 to 4. Still coarse, still no entrant at either end, but the comment now says what the data says. Worker quintile shares remain [0.198, 0.201, 0.200, 0.201, 0.200].

@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

@juaristi22 re-review requested. Since your 13:32 pass: C1's tie-ordering is fixed by blocking on the value rather than the row (ten identical adults now all get quintile 3), with two tests driving the real function through the equal-threshold branch — both fail on the old code with your [1,1,2,2,3,3,4,4,5,5]. A1 is documented rather than changed, with the bound quantified and a way to reproduce it. Are you happy with these, or is there anything still open?

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

Source Documents

Critical (Must Fix)

  1. [C1 STILL OPEN] The ordinary weighted-threshold path remains row-order dependent when equal-earning workers carry unequal weights. The latest commit fixes the all-equal fallback by aggregating tied potential values, but weighted_quantiles still sorts rows and interpolates from each row's weight midpoint without first aggregating equal values. Swapping two £10,000 workers with weights 1 and 2 changed the thresholds from [10000, 11600, 16400, 23000] to [10000, 10000, 16400, 23000]; after restoring identities, the exact-head function changed the two identical workers from Q2 to Q3. Their elasticities therefore still depend on serialization order. Aggregate weights by distinct earnings value before computing the midpoint CDF, and add a permutation test with tied earnings and unequal weights. — policyengine_uk/dynamics/participation.py:411

Should Address

  1. [A1 ACKNOWLEDGED] Full-time-equivalent placement remains a material unsupported modeling assumption. The code and changelog now state clearly that the sources do not prescribe it, quantify the approximate twofold elasticity bound, and point to #1836. That transparency is sufficient for this PR, but the assumption still needs calibration or sensitivity validation before treating the participation result as settled. — policyengine_uk/dynamics/participation.py:477
  2. [A5 NEW] The Towncrier fragment renders with a duplicated bullet. The file starts with - , while the template adds its own list marker, producing - - Place earnings quintiles.... Remove the leading dash. — changelog.d/1827.fixed.md:1

Resolved Prior Findings

  • [C1 PARTIALLY RESOLVED] The exact all-equal fallback reproducer is now tie- and order-invariant; the same invariant still fails in the ordinary weighted-threshold path described above.
  • [C2 RESOLVED] Worker-derived thresholds avoid splitting tied non-worker masses by rank.
  • [A2 RESOLVED] A no-worker population is not forced wholly into quintile 5.
  • [C3 RESOLVED] The changelog fragment is consumed by Towncrier.
  • [A3 RESOLVED] Zero/non-finite weights and pandas 3 dtype assignments are handled.
  • [A4 RESOLVED] weighted_median filters non-finite pairs and rejects mismatched shapes.

Validation Summary

  • Focused tests: 15 passed.
  • Adversarial tied-income/unequal-weight permutation reproducer: failed on the exact head ([2,2,3,4,5] versus restored [3,3,3,4,5]).
  • Ruff: passed on changed Python files.
  • Towncrier: fragment is consumed but renders a duplicated bullet.
  • Exact-head GitHub CI: all 8 checks passed; PR is mergeable.

Review Severity: REQUEST_CHANGES

Next Steps

Aggregate equal worker earnings into weighted blocks before interpolating thresholds, then add the unequal-weight permutation regression test.

Addresses review on #1829.

Ranking a pooled distribution of workers' actual and non-workers' imputed
earnings split the tied mass of non-workers across a quintile boundary, and
which member of the mass fell on which side depended on the order of rows in
the dataset. A permutation of the test fixture moved 23 of 80 adults, all of
them non-workers. Elasticities and stochastic responses should not vary with
serialisation.

Thresholds are now the weighted quintile boundaries of the observed earnings
distribution of working adults, and everyone is placed against them by value:
workers on actual earnings, non-workers on imputed potential earnings. A tied
mass lands wholly in the quintile its value belongs to, so the result is
independent of row order — verified over permutations in the tests.

This also drops the claim that the OBR note prescribes ranking workers and
non-workers together, which it does not say. Thresholds estimated from
observed earnings are a weaker and better-supported assumption.

Quintiles are consequently not equal shares of the adult population: workers
divide roughly 20% each, while non-workers concentrate where their imputed
wage falls. That is intended — these are quintiles of the earnings
distribution, which is what Table A1 is indexed on.

weighted_median now falls back to the unweighted median for a group carrying
no weight, rather than returning zero. A zero there silently bars entry into
employment, and the previous `if donors.any()` guard did not catch it: a
non-empty but weightless donor group returned zero and bypassed the fallback.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NKBpgpwwY5ZXHUwh1GJQT
@vahid-ahmadi
vahid-ahmadi force-pushed the fix-earnings-quintile-population branch from 5ede79a to 9de879b Compare August 28, 2026 14:37
@vahid-ahmadi

Copy link
Copy Markdown
Collaborator Author

@juaristi22 C1 confirmed again — you were right that I fixed the fallback and not the ordinary path. Same bug class, different function.

weighted_quantiles still sorted rows and took each row's own weight midpoint, so a tied value occupied as many positions on the CDF as it had rows, ordered by input position. It now aggregates weight by distinct value first (np.unique + np.bincount), so one value has one position however many rows carry it.

Two tests, both failing on the previous head:

  • thresholds are invariant to swapping the weights of two tied rows — previously [10000, 10000, 16666.67, 25000] vs [10000, 10000, 15000, 25000];
  • and at quintile level, earnings [10k, 10k, 15k, 22k, 22k, 26k, 40k] with weights [1, 3, 2, 2, 2, 3, 2] moved the tied pair from [1, 1, ...] to [2, 2, ...] on that swap alone.

I searched for that second fixture rather than assert one: my first attempt passed on the old code, so it would have been a test that never failed.

A5 fixed — the fragment started with - and the template adds its own marker, so it rendered - - Place earnings quintiles.... Same duplicated-bullet issue existed on the data-side fragments and is fixed there too.

On real data enhanced_frs_2024_25: worker quintile shares [0.201, 0.199, 0.200, 0.201, 0.200], non-workers still spanning 2 to 4. A1 I leave as documented, per your acknowledgement.

@juaristi22 juaristi22 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Program Review

Source Documents

Critical (Must Fix)

  • None.

Should Address

  1. [A1 STILL OPEN, ACKNOWLEDGED] Full-time-equivalent placement remains a material unsupported modeling assumption. The implementation now documents that the cited sources do not prescribe the 37.5-hour scaling, quantifies that unscaled 18.8-hour placement roughly doubles entrant elasticities, and tracks validation in #1836. This is transparent enough not to block this corrective PR, but downstream participation results should retain that sensitivity caveat. — policyengine_uk/dynamics/participation.py:484

Resolved Prior Findings

  • [C1 RESOLVED] weighted_quantiles now aggregates weights by distinct earnings value before constructing the CDF. The exact prior £10k tied-worker/unequal-weight permutation reproducer now returns identical restored quintiles ([2,2,3,4,5] both ways), and two focused regression tests cover thresholds and end-to-end placement. — policyengine_uk/dynamics/participation.py:411
  • [A5 RESOLVED] The leading dash was removed from the changelog fragment; Towncrier now renders a single bullet.
  • Earlier worker-threshold, degenerate-distribution, zero-weight, finite-value, dtype, and entity/test findings remain resolved.

Validation Summary

  • Focused tests: 17 passed.
  • Exact adversarial permutation reproducer: passed.
  • Ruff: passed on changed Python files.
  • Towncrier: Fixed fragment renders correctly.
  • Exact-head GitHub CI: all 8 checks passed; PR is mergeable.

Review Severity: APPROVE_WITH_COMMENT

Next Steps

The row-order bug is fixed. Keep the FTE sensitivity visible until #1836 supplies an empirical calibration.

@vahid-ahmadi
vahid-ahmadi merged commit 4f6506e into main Aug 28, 2026
8 checks passed
@vahid-ahmadi
vahid-ahmadi deleted the fix-earnings-quintile-population branch August 28, 2026 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

calculate_earnings_quintile ranks the whole population including children, so Q1/Q2 hold no earners

2 participants