Add labor supply response macro output#360
Conversation
|
@vahid-ahmadi, when you review this PR, could you be particularly concerned with how The key risk is that LSRs depend on a conditionally present set of variables. When a reform/dynamic activates labor-supply responses, all of those variables need to be present in the output data for LSRs to calculate correctly; when LSRs are inactive, we intentionally avoid requiring those optional columns and return the legacy zero-shaped result. I did not add a full integration test for this wiring because the full simulation runtime is long, so the current coverage is focused unit coverage around activation detection, missing-column behavior, output shape, and household-weighted decile calculations. |
0b34481 to
a37079f
Compare
PavelMakarchuk
left a comment
There was a problem hiding this comment.
Approving on methodology. Verified field-by-field against legacy policyengine-api/policyengine_api/endpoints/economy/compare.py:26-98:
| Field | Match |
|---|---|
substitution_lsr / income_lsr (scalar diffs) |
✓ |
relative_lsr = lsr_hh.sum() / original_earnings.sum() |
✓ (PR uses _safe_ratio → 0 instead of NaN/inf on zero denominator — minor robustness improvement) |
decile.average.{income,substitution} — .groupby(decile).mean(), all deciles |
✓ |
decile.relative.{income,substitution} — sum/sum per decile, filtered > 0 |
✓ |
original_earnings = baseline_earnings - total_lsr_hh (where total_lsr_hh = reform_lsr - baseline_lsr) |
✓ (preserves the quirky legacy formula) |
hours.{change, income_effect, substitution_effect} |
✓ |
revenue_change = 0.0 |
✓ (preserves legacy bug — budgetary_impact_lsr was never populated) |
The test_calculate_us_labor_supply_response_uses_legacy_shape test pins hand-computed expected values (e.g. relative_lsr.income = 40.0/285.0, decile.relative.income[1] = 20.0/165.0), which locks in the legacy numerator/denominator structure.
The activeness-gating piece is new — it skips materializing 6 (US) or 2 (UK) expensive person-level variables when neither policy nor dynamic looks LSR-related. Detection uses parameter-prefix matching (both labor/labour spellings for UK), an explicit affects_labor_supply_response field, and simulation_modifier presence as a conservative fallback. Sensible heuristic.
Non-blocking follow-ups:
revenue_changelegacy bug — please file an issue to actually compute the LSR revenue effect. Parity with the legacy zero is fine today, but it's a real user-visible gap.- Code volume — 633 lines for ~50 lines of legacy logic. The active/inactive bifurcation duplicates much of the calc; could be unified by constructing zero-filled series upfront and running one code path. Tests pin behavior so refactoring later is safe.
_calculate_hourscontrol flow — the dual-path column-probing plusactiveflag is convoluted. Likely simplifies to "if active, compute; else zeros."affects_labor_supply_response: Optional[bool]on Policy/Dynamic — new public API surface for a single use case, with a non-trivial three-way truth table in__add__. Consider whether this belongs in a metadata dict instead of a top-level field._safe_ratiovs legacy NaN/inf behavior — deliberate improvement; worth a code comment noting this as a deliberate divergence from "legacy-compatible."
Fixes #355
Summary
LaborSupplyResponseoutput model and calculator for US and UK macro analyses.labor_supply_responseinto US and UKPolicyReformAnalysisoutputs.Tests
ruff check src/policyengine/outputs/labor_supply_response.py tests/test_labor_supply_response.pyuv run --extra dev pytest -q tests/test_labor_supply_response.pygit diff --check