From 10106ad72498f95a6c1d82a2e6499b379c097251 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 10 Oct 2026 10:39:49 -0400 Subject: [PATCH 01/27] Fix Gift Aid income tax rate limits (#2253) Extend UK and eligible Scottish rate limits by gross Gift Aid, remove the net-donation allowance, and adjust ANI once without changing taxable income. Preserve CGT and zero-gift behavior, with policy and property tests plus statutory documentation. Co-Authored-By: Claude Opus 5.5 --- changelog.d/2253.fixed.md | 1 + docs/book/validation/spi-validation.ipynb | 2 +- docs/engineering/gift-aid.md | 80 ++++ .../reforms/cps/marriage_tax_reforms.py | 16 +- .../capital_gains_tax/capital_gains_tax.yaml | 30 +- .../capital_gains_tax_components.yaml | 34 +- .../gov/hmrc/income_tax/allowances.yaml | 4 +- .../allowances/allowance_integration.yaml | 12 +- .../hmrc/income_tax/allowances/gift_aid.yaml | 48 +- .../allowances/is_allowance_eligible.yaml | 10 +- .../received_allowances_earned_income.yaml | 14 +- .../gov/hmrc/income_tax/gift_aid.yaml | 304 +++++++++++++ .../reliefs/property_finance_cost_relief.yaml | 7 +- .../test_capital_gains_tax_components.py | 6 +- policyengine_uk/tests/test_gift_aid.py | 409 ++++++++++++++++++ policyengine_uk/utils/income_tax.py | 29 ++ policyengine_uk/utils/supplied_inputs.py | 1 + ...intenance_loan_candidate_benunit_income.py | 2 +- .../maintenance_loan_household_income.py | 2 +- .../income/uc_income_tax_on_earnings.py | 19 +- .../capital_gains_tax/capital_gains_tax.py | 6 +- .../hmrc/income_tax/adjusted_net_income.py | 24 +- .../hmrc/income_tax/allowances/allowances.py | 3 +- .../hmrc/income_tax/allowances/gift_aid.py | 4 +- .../allowances/pension_annual_allowance.py | 2 +- .../allowances/personal_allowance.py | 7 +- .../received_allowances_earned_income.py | 2 +- .../allowances/unused_personal_allowance.py | 3 +- .../add_rate_earned_income.py | 6 +- .../basic_rate_earned_income.py | 4 +- .../earned_taxable_income.py | 2 +- .../higher_rate_earned_income.py | 6 +- .../bracketized_liability/tax_band.py | 43 +- .../add_rate_savings_income.py | 8 +- .../basic_rate_savings_income.py | 6 +- .../basic_rate_savings_income_pre_starter.py | 6 +- .../higher_rate_savings_income.py | 10 +- .../gov/hmrc/income_tax/earned_income_tax.py | 17 +- .../liability/dividend_income_tax.py | 10 +- .../liability/property_income_tax.py | 28 +- .../gov/hmrc/income_tax/net_income.py | 37 ++ .../property_finance_costs_relieved.py | 2 +- .../student_loans/plan_2_interest_rate.py | 2 +- .../student_loans/student_loan_repayment.py | 2 +- 44 files changed, 1099 insertions(+), 171 deletions(-) create mode 100644 changelog.d/2253.fixed.md create mode 100644 docs/engineering/gift-aid.md create mode 100644 policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml create mode 100644 policyengine_uk/tests/test_gift_aid.py create mode 100644 policyengine_uk/utils/income_tax.py create mode 100644 policyengine_uk/variables/gov/hmrc/income_tax/net_income.py diff --git a/changelog.d/2253.fixed.md b/changelog.d/2253.fixed.md new file mode 100644 index 0000000000..cfccd9a421 --- /dev/null +++ b/changelog.d/2253.fixed.md @@ -0,0 +1 @@ +- Fixed Gift Aid income tax relief by extending rate limits instead of deducting donations from taxable income, and deducting grossed-up gifts from adjusted net income. diff --git a/docs/book/validation/spi-validation.ipynb b/docs/book/validation/spi-validation.ipynb index 6889042648..aac183e5c1 100644 --- a/docs/book/validation/spi-validation.ipynb +++ b/docs/book/validation/spi-validation.ipynb @@ -55,7 +55,7 @@ "1. Cases where an individual appears to transfer a portion of their Personal Allowance to their spouse as part of the Marriage Allowance. This is because the SPI removes the transferred amount from the individual's Personal Allowance, but does not indicate that the subsequent lower amount is driven by the Marriage Allowance. This may represent as much as 37,998 records (4.8% of total).\n", "1. Cases where an individual receives less than £100,000 in annual income and is non-resident for taxation purposes. In this situation, the individual receives £0 Personal Allowance, even though our model would project a full £12,500 allowance. The SPI uses a variable that is not publicly available to calculate this status. This represents 2,054 records (0.3% of total).\n", "1. Cases where both the SPI and PolicyEngine calculate the same amount of taxable savings income for an individual receiving any form of savings income classified by the SPI as \"Other Investment Income\" (e.g., interest on securities), but the SPI finds that they owe around £1,000 more in income tax than we do. This represents 1,276 records (0.2% of total).\n", - "1. Cases where an individual near the Personal Allowance taper value who claims Gift Aid receives a full Personal Allowance of £12,500 within the SPI model. The SPI model's Gift Aid variable signifies the amount an individual deducts due to Gift Aid donations, while PolicyEngine's represents an individual's total Gift Aid-eligible donations. We then calculate the individual's deduction amount from this value. This error represents 738 records (0.1% of total).\n", + "1. Cases where an individual near the Personal Allowance taper value who claims Gift Aid receives a full Personal Allowance of £12,500 within the SPI model. This historical validation identified 738 records (0.1% of total) with differences associated with the interpretation of Gift Aid inputs. PolicyEngine's `gift_aid` input is the net eligible donation. Under [ITA 2007 ss. 414–415](https://www.legislation.gov.uk/ukpga/2007/3/section/414), the grossed-up gift extends the income tax rate limits; it does not reduce taxable income. The grossed-up gift also reduces adjusted net income for the Personal Allowance taper under [s. 58](https://www.legislation.gov.uk/ukpga/2007/3/section/58). These figures describe the original validation and have not been recalculated after that treatment was corrected.\n", "\n", "## Conclusion\n", "PolicyEngine's UK model shows a high level of accordance with the SPI's 2020/21 data, with almost 93% of records with the same inputs returning the same total income tax calculation. While some discrepancies exist, they are limited in scope and often attributable to either testability limitations within the SPI's structure or to inputs that the SPI has not made publicly available. This validation exercise builds upon our previous validations, such as our [Autumn 2023 model calibration](https://policyengine.org/uk/research/uk-calibration-2023-q4). We welcome questions and comments on our findings or methodology - please feel free to [get in touch](mailto:contact@policyengine.org)." diff --git a/docs/engineering/gift-aid.md b/docs/engineering/gift-aid.md new file mode 100644 index 0000000000..b64ef562c3 --- /dev/null +++ b/docs/engineering/gift-aid.md @@ -0,0 +1,80 @@ +# Gift Aid and income tax + +`gift_aid` is the net eligible donation. [Income Tax Act 2007 +s. 414(2)(a)](https://www.legislation.gov.uk/ukpga/2007/3/section/414) +treats the gift as made after deduction of basic-rate income tax, and +[s. 415](https://www.legislation.gov.uk/ukpga/2007/3/section/415) defines its +grossed-up amount. The model calculates `gift_aid_grossed_up` as the net gift +divided by one minus the UK basic rate. + +Gift Aid does not belong in `allowances` and does not reduce taxable income. +Under s. 414(2)(b), the gross gift increases both UK rate limits. For Scottish +non-savings income, it also increases the Scottish basic-rate upper limit and +every limit above it; the starter-rate upper limit remains unchanged. Savings +and dividends use the extended UK limits for Scottish taxpayers too. + +The model extends limits where band amounts are calculated. For variables that +use a complete rate scale, `calc_income_tax_with_gift_aid` in +`policyengine_uk/utils/income_tax.py` applies an equivalent correction to the +scale's original calculation. At each extended threshold, it subtracts the +rate increase multiplied by the income between the old and extended limits. +For a stacked slice, subtracting the corrected tax at its bottom from the +corrected tax at its top gives the tax on that slice. This preserves the +original scale calculation when the gift is zero and handles gifts that cross +several thresholds. + +`net_income` holds the income base previously exposed as `adjusted_net_income`. +Taxable-income calculations and allowance allocation use this base. +When adjusted net income is supplied explicitly, its gross Gift Aid adjustment +is added back to obtain the taxable-income base. This preserves direct income +inputs when the gift is zero without recovering ordinary taxable income from a +rounded adjusted-net-income calculation. The marriage-neutral income-tax +reform splits `net_income` before each person's Gift Aid adjustment. +`adjusted_net_income` deducts the gross gift once, as [s. 58(1), Step +2](https://www.legislation.gov.uk/ukpga/2007/3/section/58) requires. The Personal +Allowance taper and High Income Child Benefit Charge therefore see the lower +adjusted net income. The Personal Allowance formula does not deduct Gift Aid a +second time. Capital gains tax uses `net_income`, retains its gross Gift Aid +band extension, and no longer removes the net gift from `allowances`. + +## Existing limitations + +Zero-gift calculations retain their existing behavior. Several inherited +limitations remain relevant when interpreting mixed-income results: + +- Savings and dividend stacking currently omits property income from the + income beneath those slices. Gift Aid extends the limits of those existing + stacks. [ITA 2007 s. 16](https://www.legislation.gov.uk/ukpga/2007/3/section/16) + requires savings and dividends above other income; repairing property + occupancy separately would also change zero-gift results. +- The Personal Savings Allowance follows the model's `tax_band` enum, including + its Scottish band classification. [s. 12B(8)](https://www.legislation.gov.uk/ukpga/2007/3/section/12B) + instead tests Scottish non-savings income against the counterfactual UK + higher and additional rates. Extending the existing band tests preserves + that limitation. +- The Scottish top-rate threshold parameter is £112,570 of income after + allowances for 2024 onwards. The [HMRC rates and allowances + table](https://www.gov.uk/government/publications/rates-and-allowances-income-tax/income-tax-rates-and-allowances-current-and-past) + gives £125,140 after allowances. The Gift Aid correction extends the model's + existing threshold and does not repair it. +- Adjusted net income still omits other adjustments in s. 58, including the + deduction for pension contributions relieved at source in Step 3. +- Gifts are assumed to qualify. The model does not charge the donor for a + shortfall in income tax and capital gains tax covering the tax attributed to + gifts under [ss. 423–424](https://www.legislation.gov.uk/ukpga/2007/3/section/424). + +## Relief bounds in tests + +For a fixed taxable-income stack and fixed zero-rate allowances, the maximum +band-extension relief is the gross gift multiplied by the sum of positive +rate increases at extended limits. For earned income, that cumulative rate +increase is the top rate minus the basic rate: 25 percentage points for the UK +schedule and 28 for the current Scottish schedule. A bound using only the +largest adjacent rate increase would fail even for the £150,000 UK example, +where both UK limits move. + +Restoring Personal Allowance through the adjusted-net-income taper can give +additional relief. Changing tax band can also increase the Personal Savings +Allowance discretely under s. 12B, so its relief cannot be bounded by a constant +times a very small gift. Tests of the continuous band-extension bound must +allow for that allowance change or hold the zero-rate allowance fixed. diff --git a/policyengine_uk/reforms/cps/marriage_tax_reforms.py b/policyengine_uk/reforms/cps/marriage_tax_reforms.py index 813cfc4570..73efa66e79 100644 --- a/policyengine_uk/reforms/cps/marriage_tax_reforms.py +++ b/policyengine_uk/reforms/cps/marriage_tax_reforms.py @@ -153,9 +153,11 @@ def formula(person, period, parameters): COMPONENTS.append("basic_income") return max_(0, add(person, period, COMPONENTS)) - class adjusted_net_income(Variable): - label = "Optimised adjusted net income" - documentation = "Adjusted net income, but split equally between partners if they are married" + class net_income(Variable): + label = "Optimised net income" + documentation = ( + "Income before Gift Aid adjustments, split equally between married partners" + ) entity = Person definition_period = YEAR value_type = float @@ -171,9 +173,7 @@ def formula(person, period, parameters): originally_split_income_branch = person.simulation.get_branch( "originally_split_income", clone_system=True ) - originally_split_income_branch.set_input( - "adjusted_net_income", period, income - ) + originally_split_income_branch.set_input("net_income", period, income) originally_split_income_tax = person.benunit.sum( originally_split_income_branch.calculate("income_tax", period) ) @@ -186,7 +186,7 @@ def formula(person, period, parameters): split_income_branch = person.simulation.get_branch( "split_income", clone_system=True ) - split_income_branch.set_input("adjusted_net_income", period, split_income) + split_income_branch.set_input("net_income", period, split_income) split_income_tax = person.benunit.sum( split_income_branch.calculate("income_tax", period) ) @@ -200,7 +200,7 @@ def formula(person, period, parameters): class reform(Reform): def apply(self): self.add_variable(meets_ma_neutral_tax_conditions) - self.update_variable(adjusted_net_income) + self.update_variable(net_income) self.add_variable(unadjusted_net_income) return reform diff --git a/policyengine_uk/tests/policy/baseline/gov/hmrc/capital_gains_tax/capital_gains_tax.yaml b/policyengine_uk/tests/policy/baseline/gov/hmrc/capital_gains_tax/capital_gains_tax.yaml index 635a946d7c..aa94221356 100644 --- a/policyengine_uk/tests/policy/baseline/gov/hmrc/capital_gains_tax/capital_gains_tax.yaml +++ b/policyengine_uk/tests/policy/baseline/gov/hmrc/capital_gains_tax/capital_gains_tax.yaml @@ -6,7 +6,7 @@ - name: No capital gains period: 2025 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 0 output: capital_gains_tax: 0 @@ -14,7 +14,7 @@ - name: Capital gains below annual exempt amount period: 2025 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 2_000 output: capital_gains_tax: 0 @@ -22,7 +22,7 @@ - name: Capital gains just above annual exempt amount period: 2025 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 3_500 output: capital_gains_tax: 90 @@ -30,7 +30,7 @@ - name: Basic rate capital gains tax period: 2025 input: - adjusted_net_income: 20_000 + net_income: 20_000 capital_gains: 23_000 output: capital_gains_tax: 3_600 @@ -39,7 +39,7 @@ period: 2025 absolute_error_margin: 0.01 input: - adjusted_net_income: 50_000 + net_income: 50_000 capital_gains: 50_000 output: capital_gains_tax: 11_263.80 @@ -47,7 +47,7 @@ - name: Additional rate capital gains tax period: 2025 input: - adjusted_net_income: 150_000 + net_income: 150_000 capital_gains: 100_000 output: capital_gains_tax: 23_280 @@ -56,7 +56,7 @@ period: 2025 absolute_error_margin: 0.01 input: - adjusted_net_income: 35_000 + net_income: 35_000 capital_gains: 40_000 output: capital_gains_tax: 7_963.80 @@ -64,7 +64,7 @@ - name: High capital gains, low income period: 2025 input: - adjusted_net_income: 10_000 + net_income: 10_000 capital_gains: 200_000 output: capital_gains_tax: 45_018 @@ -73,7 +73,7 @@ period: 2025 absolute_error_margin: 0.1 input: - adjusted_net_income: 500_000 + net_income: 500_000 capital_gains: 1_000_000 output: capital_gains_tax: 239_280 @@ -81,7 +81,7 @@ - name: Edge case, capital gains exactly equal to annual exempt amount period: 2025 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 3_000 output: capital_gains_tax: 0 @@ -105,7 +105,7 @@ input: people: person1: - adjusted_net_income: 9_324 + net_income: 9_324 capital_gains: 228_890 person2: capital_gains: -124_972 @@ -117,7 +117,7 @@ - name: Case 13, Gift Aid extends the basic rate band for capital gains. period: 2018 input: - adjusted_net_income: 41_850 + net_income: 41_850 gift_aid: 40_000 capital_gains: 70_000 output: @@ -136,7 +136,7 @@ period: 2026 absolute_error_margin: 0.01 input: - adjusted_net_income: 11_336.06 + net_income: 11_336.06 capital_gains: 32_942.86 output: capital_gains_tax: 5_389.71 @@ -150,7 +150,7 @@ period: 2024 absolute_error_margin: 0.01 input: - adjusted_net_income: 20_000 + net_income: 20_000 capital_gains: 23_000 output: capital_gains_tax: 2_692.60 @@ -159,7 +159,7 @@ period: 2024 absolute_error_margin: 0.01 input: - adjusted_net_income: 150_000 + net_income: 150_000 capital_gains: 100_000 output: capital_gains_tax: 21_079.56 diff --git a/policyengine_uk/tests/policy/baseline/gov/hmrc/capital_gains_tax/capital_gains_tax_components.yaml b/policyengine_uk/tests/policy/baseline/gov/hmrc/capital_gains_tax/capital_gains_tax_components.yaml index f7676af409..9f7b32c837 100644 --- a/policyengine_uk/tests/policy/baseline/gov/hmrc/capital_gains_tax/capital_gains_tax_components.yaml +++ b/policyengine_uk/tests/policy/baseline/gov/hmrc/capital_gains_tax/capital_gains_tax_components.yaml @@ -11,13 +11,13 @@ # The residential property higher rate is 24% for the whole of 2024-25. - name: Residential property 2024, 18% on the unused basic rate band and 24% above, after one AEA. - # adjusted_net_income 42,570 less the 12,570 personal allowance leaves taxable + # net_income 42,570 less the 12,570 personal allowance leaves taxable # income 30,000 and 7,700 of the 37,700 basic rate band. 100,000 - 3,000 AEA = # 97,000: 7,700 x 0.18 = 1,386 plus 89,300 x 0.24 = 21,432. period: 2024 absolute_error_margin: 0.01 input: - adjusted_net_income: 42_570 + net_income: 42_570 capital_gains: 100_000 capital_gains_residential_property: 100_000 output: @@ -29,7 +29,7 @@ period: 2024 absolute_error_margin: 0.01 input: - adjusted_net_income: 42_570 + net_income: 42_570 capital_gains_before_response: 100_000 capital_gains_residential_property: 100_000 output: @@ -39,7 +39,7 @@ # 500,000 - 3,000 AEA = 497,000 x 0.10. period: 2024 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 500_000 capital_gains_badr: 500_000 output: @@ -49,7 +49,7 @@ # 497,000 x 0.14 (Finance Act 2025 s. 8, disposals from 6 April 2025). period: 2025 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 500_000 capital_gains_badr: 500_000 output: @@ -59,7 +59,7 @@ # 497,000 x 0.18 (Finance Act 2025 s. 8, disposals from 6 April 2026). period: 2026 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 500_000 capital_gains_badr: 500_000 output: @@ -73,7 +73,7 @@ period: 2025 absolute_error_margin: 0.1 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 1_500_000 capital_gains_badr: 1_500_000 output: @@ -85,7 +85,7 @@ period: 2024 absolute_error_margin: 0.1 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 1_010_000 capital_gains_badr: 1_010_000 output: @@ -97,7 +97,7 @@ period: 2024 absolute_error_margin: 0.1 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 1_000_000 capital_gains_carried_interest: 1_000_000 output: @@ -108,7 +108,7 @@ period: 2025 absolute_error_margin: 0.1 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 1_000_000 capital_gains_carried_interest: 1_000_000 output: @@ -122,7 +122,7 @@ # the residential gains would have paid 0.18 and the total would be 6,538. period: 2025 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 47_700 capital_gains_badr: 37_700 capital_gains_residential_property: 10_000 @@ -141,7 +141,7 @@ period: 2024 absolute_error_margin: 0.01 input: - adjusted_net_income: 20_000 + net_income: 20_000 capital_gains: 100_000 capital_gains_badr: 20_000 capital_gains_carried_interest: 10_000 @@ -155,7 +155,7 @@ # Residential: 37,700 x 0.18 = 6,786 plus 2,300 x 0.24 = 552. period: 2025 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 50_000 capital_gains_residential_property: 40_000 capital_gains_carried_interest: 30_000 @@ -167,7 +167,7 @@ # 34,500 x 0.18 = 6,210 plus 3,800 x 0.28 = 1,064. period: 2018 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 50_000 capital_gains_residential_property: 50_000 output: @@ -176,7 +176,7 @@ - name: A schedule input without capital_gains is ignored. period: 2025 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: 0 capital_gains_residential_property: 50_000 output: @@ -185,7 +185,7 @@ - name: A net loss pays nothing whatever the schedule inputs say. period: 2025 input: - adjusted_net_income: 0 + net_income: 0 capital_gains: -20_000 capital_gains_residential_property: 50_000 output: @@ -196,7 +196,7 @@ period: 2025 absolute_error_margin: 0.01 input: - adjusted_net_income: 50_000 + net_income: 50_000 capital_gains: 50_000 capital_gains_badr: 0 capital_gains_residential_property: 0 diff --git a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances.yaml b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances.yaml index 6f2f1f0f16..970f0d71f1 100644 --- a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances.yaml +++ b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances.yaml @@ -30,14 +30,14 @@ period: 2020 absolute_error_margin: 0 input: - adjusted_net_income: 10000 + net_income: 10000 output: pension_annual_allowance: 40000 - name: Annual Allowance reduced period: 2020 absolute_error_margin: 0 input: - adjusted_net_income: 320000 + net_income: 320000 output: pension_annual_allowance: 4000 - name: Trading allowance full deduction diff --git a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/allowance_integration.yaml b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/allowance_integration.yaml index 07cc84943b..f9fbe29a6a 100644 --- a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/allowance_integration.yaml +++ b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/allowance_integration.yaml @@ -1,23 +1,23 @@ -- name: Income tax, based on adjusted net income +- name: Income tax, based on net income period: 2024 absolute_error_margin: 4 input: - adjusted_net_income: 40000 + net_income: 40000 output: income_tax: 5484.2 -- name: Income tax, increasing adjusted net income +- name: Income tax, increasing net income period: 2024 absolute_error_margin: 4 input: - adjusted_net_income: 50000 + net_income: 50000 output: income_tax: 7484.2 -- name: Income tax, continously increasing adjusted net income +- name: Income tax, continuously increasing net income period: 2024 absolute_error_margin: 4 input: - adjusted_net_income: 60000 + net_income: 60000 output: income_tax: 11428.4 diff --git a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/gift_aid.yaml b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/gift_aid.yaml index 921f9fb2fa..233dc97b68 100644 --- a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/gift_aid.yaml +++ b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/gift_aid.yaml @@ -1,29 +1,31 @@ -# Gift Aid tests -# ITA 2007 s.58 requires grossed-up Gift Aid to be deducted from ANI -# when calculating Personal Allowance taper +# ITA 2007 ss.414(2)(b), 415 extend the basic and higher rate limits by +# the grossed-up gift rather than deducting the net gift from taxable income. +# https://www.legislation.gov.uk/ukpga/2007/3/section/414 +# https://www.legislation.gov.uk/ukpga/2007/3/section/415 +# s.58 Step 2 deducts the gross gift from adjusted net income (ANI). +# https://www.legislation.gov.uk/ukpga/2007/3/section/58 -- name: Higher rate taxpayer gets 40% relief +- name: Higher-rate taxpayer gets relief from the extended basic-rate limit period: 2025 - absolute_error_margin: 1 + absolute_error_margin: 0.01 input: employment_income: 60000 gift_aid: 1000 output: - # Basic rate band is £37,700, PA is £12,570 - # Higher rate threshold = £50,270 - # £60k income means in 40% band - # £1k gift aid reduces taxable income by £1k, saving 40% - income_tax: 11032 + # £1,000 net = £1,250 gross, moving £1,250 from 40% to 20%. + # £11,432 tax without Gift Aid less £250 relief. + income_tax: 11182 -- name: Basic rate taxpayer gets 20% relief +- name: Basic-rate taxpayer gets no further taxpayer relief period: 2025 - absolute_error_margin: 1 + absolute_error_margin: 0.01 input: employment_income: 30000 gift_aid: 1000 output: - # In basic rate band, £1k gift aid saves 20% - income_tax: 3286 + # s.414(2)(a) treats the gift as made after basic-rate deduction. + # Extending the basic-rate limit leaves this taxpayer's tax unchanged. + income_tax: 3486 - name: Gift Aid reduces ANI for PA taper - full PA restored period: 2025 @@ -52,20 +54,14 @@ # PA = £12,570 - £5k = £7,570 personal_allowance: 7570 -- name: PA taper zone effective relief is ~60% +- name: Gift Aid restores personal allowance and extends rate limits period: 2025 - absolute_error_margin: 50 + absolute_error_margin: 0.01 input: employment_income: 110000 gift_aid: 10000 output: - # Without gift aid: PA = £7,570, tax ~£33,432 - # With £10k gift aid (£12.5k gross): - # - ANI for PA taper reduced to £97,500 (below £100k) - # - Full PA £12,570 restored - # - Taxable = £110k - £12,570 (PA) - £10k (gift aid) = £87,430 - # - Tax at basic rate (£37,700): £7,540 - # - Tax at higher rate (£87,430 - £37,700 = £49,730): £19,892 - # - Total: £27,432 - # Tax savings: ~£6,000 = 60% effective relief on £10k donation - income_tax: 27432 + # £10,000 net = £12,500 gross; ANI £97,500 restores PA £12,570. + # Taxable income remains £110,000 - £12,570 = £97,430. + # £50,200 at 20% plus £47,230 at 40% = £28,932. + income_tax: 28932 diff --git a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/is_allowance_eligible.yaml b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/is_allowance_eligible.yaml index 9d8e883419..a91dfaee06 100644 --- a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/is_allowance_eligible.yaml +++ b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/is_allowance_eligible.yaml @@ -30,10 +30,10 @@ - name: Annual Allowance for Pension Contributions period: 2024 input: - adjusted_net_income: 60_000 + net_income: 60_000 output: # AA = £60,000 from 2023-04-06 (FA 2004 s.228, amended by Finance (No. 2) Act 2023) - # ANI of £60,000 is below the taper threshold of £260,000, so full AA applies + # Net income of £60,000 is below the taper threshold of £260,000, so full AA applies pension_annual_allowance: 60_000 - name: Trading Allowance Deduction @@ -60,7 +60,7 @@ output: savings_allowance: 1000.0 -- name: Allowances Applicable to Adjusted Net Income +- name: Allowances applicable to net income exclude Gift Aid period: 2024 input: personal_allowance: 12000 @@ -70,4 +70,6 @@ charitable_investment_gifts: 100 other_deductions: 50 output: - allowances: 14950.0 + # ITA 2007 s.414(2)(b) extends limits instead of deducting the gift. + # https://www.legislation.gov.uk/ukpga/2007/3/section/414 + allowances: 14450.0 diff --git a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/received_allowances/received_allowances_earned_income.yaml b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/received_allowances/received_allowances_earned_income.yaml index 3f9c06ad8b..6acebb4035 100644 --- a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/received_allowances/received_allowances_earned_income.yaml +++ b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/allowances/received_allowances/received_allowances_earned_income.yaml @@ -2,7 +2,7 @@ period: 2024 input: allowances: 12_570 - adjusted_net_income: 10_000 + net_income: 10_000 taxable_savings_interest_income: 500 taxable_dividend_income: 500 output: @@ -12,7 +12,7 @@ period: 2024 input: allowances: 12_570 - adjusted_net_income: 13_570 + net_income: 13_570 taxable_savings_interest_income: 500 taxable_dividend_income: 500 output: @@ -22,7 +22,7 @@ period: 2024 input: allowances: 12_570 - adjusted_net_income: 50_000 + net_income: 50_000 taxable_savings_interest_income: 2_000 taxable_dividend_income: 3_000 output: @@ -32,7 +32,7 @@ period: 2024 input: allowances: 12_570 - adjusted_net_income: 20_000 + net_income: 20_000 taxable_savings_interest_income: 8_000 taxable_dividend_income: 7_000 output: @@ -42,17 +42,17 @@ period: 2024 input: allowances: 12_570 - adjusted_net_income: 30_000 + net_income: 30_000 taxable_savings_interest_income: 0 taxable_dividend_income: 0 output: received_allowances_earned_income: 12_570 -- name: Adjusted net income is erroneously negative +- name: Net income is erroneously negative period: 2024 input: allowances: 12_570 - adjusted_net_income: -10_000 + net_income: -10_000 taxable_savings_interest_income: 0 taxable_dividend_income: 0 output: diff --git a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml new file mode 100644 index 0000000000..43124e502e --- /dev/null +++ b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml @@ -0,0 +1,304 @@ +# ITA 2007 s.414(2)(a)-(b): the net gift is not a taxable-income +# deduction; the grossed-up gift extends rate limits. +# https://www.legislation.gov.uk/ukpga/2007/3/section/414 +# s.415: a net £800 gift is £1,000 gross at the 20% UK basic rate. +# https://www.legislation.gov.uk/ukpga/2007/3/section/415 +# s.58 Step 2: deduct the gross gift from adjusted net income (ANI). +# https://www.legislation.gov.uk/ukpga/2007/3/section/58 + +- name: Gift Aid gives no income tax relief to a UK basic-rate earner + period: 2026 + absolute_error_margin: 0.01 + input: + age: [40, 40] + region: [NORTH_WEST, NORTH_WEST] + employment_income: [30_000, 30_000] + gift_aid: [0, 800] + output: + gift_aid_grossed_up: [0, 1_000] + adjusted_net_income: [30_000, 29_000] + earned_taxable_income: [17_430, 17_430] + # £17,430 at 20%, with no taxpayer relief on the Gift Aid gift. + income_tax: [3_486, 3_486] + uc_income_tax_on_earnings: [3_486, 3_486] + # UC deducts the tax actually due on earnings, plus £1,394.40 NI. + uc_individual_earned_income_before_mif: [25_119.60, 25_119.60] + +- name: Gift Aid gives £200 relief to a UK higher-rate earner + period: 2026 + absolute_error_margin: 0.01 + input: + age: [40, 40] + region: [NORTH_WEST, NORTH_WEST] + employment_income: [60_000, 60_000] + gift_aid: [0, 800] + output: + earned_taxable_income: [47_430, 47_430] + # £1,000 moves from 40% to 20% under s.414(2)(b). + income_tax: [11_432, 11_232] + uc_income_tax_on_earnings: [11_432, 11_232] + uc_individual_earned_income_before_mif: [45_357.40, 45_557.40] + +- name: Gift Aid gives £250 relief to a UK additional-rate earner + period: 2026 + absolute_error_margin: 0.01 + input: + age: [40, 40] + region: [NORTH_WEST, NORTH_WEST] + employment_income: [150_000, 150_000] + gift_aid: [0, 800] + output: + earned_taxable_income: [150_000, 150_000] + # £1,000 moves from 40% to 20% and £1,000 from 45% to 40%. + income_tax: [53_703, 53_453] + uc_income_tax_on_earnings: [53_703, 53_453] + uc_individual_earned_income_before_mif: [91_286.40, 91_536.40] + +- name: Gift Aid extends Scottish basic and higher limits + period: 2026 + absolute_error_margin: 0.02 + input: + age: [40, 40] + region: [SCOTLAND, SCOTLAND] + employment_income: [60_000, 60_000] + gift_aid: [0, 800] + output: + # £1,000 moves from 21% to 20% and £1,000 from 42% to 21%. + # The Scottish starter/basic boundary is not extended by s.414. + income_tax: [13_182.05, 12_962.05] + +- name: Gift Aid restores £500 of personal allowance at £101000 income + period: 2026 + absolute_error_margin: 0.01 + input: + age: [40, 40] + region: [NORTH_WEST, NORTH_WEST] + employment_income: [101_000, 101_000] + gift_aid: [0, 800] + output: + adjusted_net_income: [101_000, 100_000] + # ITA 2007 s.58 Step 2 reduces the taper base exactly once. + personal_allowance: [12_070, 12_570] + earned_taxable_income: [88_930, 88_430] + income_tax: [28_032, 27_632] + +- name: Gift Aid extends UK limits for dividends above the basic-rate limit + period: 2026 + absolute_error_margin: 0.01 + input: + age: [40, 40] + region: [NORTH_WEST, NORTH_WEST] + employment_income: [50_000, 50_000] + dividend_income: [10_000, 10_000] + gift_aid: [0, 800] + output: + earned_taxable_income: [37_430, 37_430] + # The £500 dividend allowance still occupies the band. The extension + # moves £770 of taxed dividends from 35.75% to 10.75%: £192.50 relief. + dividend_income_tax: [3_396.25, 3_203.75] + income_tax: [10_882.25, 10_689.75] + +- name: Gift Aid extends UK limits across savings and dividend slices + period: 2026 + absolute_error_margin: 0.01 + input: + age: [40, 40] + region: [NORTH_WEST, NORTH_WEST] + employment_income: [49_070, 49_070] + savings_interest_income: [2_000, 2_000] + dividend_income: [5_000, 5_000] + gift_aid: [0, 1_600] + output: + # £2,000 gross extension moves £800 taxed savings down 20 percentage + # points and £700 taxed dividends down 25 percentage points. + savings_allowance: [500, 500] + savings_income_tax: [460, 300] + dividend_income_tax: [1_608.75, 1_433.75] + income_tax: [9_368.75, 9_033.75] + +- name: Gift Aid extends UK savings and dividend limits for Scottish taxpayers + period: 2026 + absolute_error_margin: 0.02 + input: + age: [40, 40] + region: [SCOTLAND, SCOTLAND] + employment_income: [50_000, 50_000] + savings_interest_income: [2_000, 2_000] + dividend_income: [5_000, 5_000] + gift_aid: [0, 800] + output: + earned_income_tax: [8_982.05, 8_762.05] + savings_income_tax: [600, 446] + dividend_income_tax: [1_608.75, 1_608.75] + income_tax: [11_190.80, 10_816.80] + +- name: Gift Aid extends the basic-rate limit where property occupies the band + period: 2026 + absolute_error_margin: 0.01 + input: + age: [40, 40] + region: [NORTH_WEST, NORTH_WEST] + employment_income: [40_000, 40_000] + property_income: [12_000, 12_000] + property_allowance_deduction: [0, 0] + gift_aid: [0, 800] + output: + earned_taxable_income: [27_430, 27_430] + property_income_tax: [2_746, 2_546] + income_tax: [8_232, 8_032] + +- name: Gift Aid extends Scottish limits where property occupies the higher band + period: 2026 + absolute_error_margin: 0.02 + input: + age: [40, 40] + region: [SCOTLAND, SCOTLAND] + employment_income: [40_000, 40_000] + property_income: [20_000, 20_000] + property_allowance_deduction: [0, 0] + gift_aid: [0, 800] + output: + # Scottish basic-limit relief falls on employment; higher-limit relief + # falls on property. Their total equals the Scottish earned-only case. + earned_income_tax: [5_551.07, 5_541.07] + property_income_tax: [7_630.98, 7_420.98] + income_tax: [13_182.05, 12_962.05] + +- name: Gift Aid extends the UK higher-rate limit for dividends + period: 2026 + absolute_error_margin: 0.02 + input: + age: [40, 40] + region: [NORTH_WEST, NORTH_WEST] + employment_income: [120_000, 120_000] + dividend_income: [40_000, 40_000] + gift_aid: [0, 800] + output: + # The basic-limit extension gives £200 earned relief. The higher-limit + # extension moves £1,000 dividends from 39.35% to 35.75%: £36 relief. + earned_income_tax: [40_460, 40_260] + dividend_income_tax: [15_376.21, 15_340.21] + income_tax: [55_836.21, 55_600.21] + +- name: Gift Aid leaves the Scottish starter-rate limit unchanged + period: 2026 + absolute_error_margin: 0.01 + input: + age: [40, 40] + region: [SCOTLAND, SCOTLAND] + employment_income: [18_000, 18_000] + gift_aid: [0, 800] + output: + # £3,967 at 19% and £1,463 at 20%, with the same starter limit. + income_tax: [1_046.33, 1_046.33] + +- name: Gift Aid extends every Scottish limit above the Scottish basic rate + period: 2026 + absolute_error_margin: 0.02 + input: + age: [40, 40] + region: [SCOTLAND, SCOTLAND] + employment_income: [150_000, 150_000] + gift_aid: [0, 800] + output: + # Rate steps 1%, 21%, 3% and 3% on £1,000 gross give £280 relief. + income_tax: [60_011.45, 59_731.45] + +- name: A large Gift Aid gift can cross several Scottish bands + period: 2026 + absolute_error_margin: 0.02 + input: + age: 40 + region: SCOTLAND + employment_income: 150_000 + gift_aid: 80_000 + output: + adjusted_net_income: 50_000 + personal_allowance: 12_570 + # Taxable £137,430 is split at £3,967, £116,956 and £131,092. + income_tax: 28_982.05 + +- name: Gift Aid can change the band used for the personal savings allowance + period: 2026 + absolute_error_margin: 0.01 + input: + age: [40, 40] + region: [NORTH_WEST, NORTH_WEST] + employment_income: [50_300, 50_300] + savings_interest_income: [100, 100] + gift_aid: [0, 800] + output: + # Tax band remains based on all taxable income as in the existing model, + # with s.414(2)(b)'s extended UK basic-rate limit (ITA 2007 s.12B). + tax_band: [HIGHER, BASIC] + savings_allowance: [500, 1_000] + savings_income_tax: [0, 0] + +- name: Gift Aid reduces adjusted net income for the Child Benefit tax charge + period: 2026 + absolute_error_margin: 0.01 + input: + age: [40, 40] + region: [NORTH_WEST, NORTH_WEST] + employment_income: [62_000, 62_000] + gift_aid: [0, 800] + child_benefit: [1_000, 1_000] + is_higher_earner: [true, true] + output: + # ITA 2007 s.58 Step 2 applies beyond the personal allowance taper. + adjusted_net_income: [62_000, 61_000] + CB_HITC: [100, 50] + +- name: Grossed-up Gift Aid can make adjusted net income negative + period: 2026 + absolute_error_margin: 0.01 + input: + age: 40 + region: NORTH_WEST + employment_income: 10_000 + gift_aid: 20_000 + # Capital Gains Tax can cover the tax attributed to a qualifying gift + # even where the gift exceeds this year's income (ITA 2007 s.423). + # https://www.legislation.gov.uk/ukpga/2007/3/section/423 + capital_gains: 50_000 + output: + # ITA 2007 s.58 Step 2 subtracts the gross gift without a floor at nil. + net_income: 10_000 + gift_aid_grossed_up: 25_000 + adjusted_net_income: -15_000 + personal_allowance: 12_570 + income_tax: 0 + +- name: Zero Gift Aid preserves the marriage-neutral income tax reform + period: 2026 + absolute_error_margin: 0.01 + reforms: + - policyengine_uk.reforms.cps.marriage_tax_reforms.marriage_neutral_it_reform + input: + people: + earner: + age: 40 + employment_income: 100_000 + gift_aid: 0 + personal_allowance: 12_570 + marriage_allowance: 0 + spouse: + age: 40 + employment_income: 0 + gift_aid: 0 + personal_allowance: 12_570 + marriage_allowance: 0 + benunits: + benunit: + members: [earner, spouse] + is_married: true + households: + household: + members: [earner, spouse] + region: NORTH_WEST + output: + # The reform pools £100,000 and splits the pre-Gift-Aid income equally. + net_income: [50_000, 50_000] + adjusted_net_income: [50_000, 50_000] + earned_taxable_income: [37_430, 37_430] + income_tax: [7_486, 7_486] diff --git a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/reliefs/property_finance_cost_relief.yaml b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/reliefs/property_finance_cost_relief.yaml index 4cae4c3dda..b205bb7efd 100644 --- a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/reliefs/property_finance_cost_relief.yaml +++ b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/reliefs/property_finance_cost_relief.yaml @@ -106,9 +106,10 @@ # deducted at Step 2 or Step 3. property_finance_costs_relieved: 5_430 property_finance_costs_carried_forward: 2_570 - # 20% x 5,430 = 1,086, limited to the Step 5 tax, which the model works - # out with Gift Aid deducted: (18,000 - 12,570 - 3,000) x 20% = 486. - property_finance_cost_relief: 486 + # ITA 2007 s.414(2)(b) extends limits without deducting Gift Aid: + # https://www.legislation.gov.uk/ukpga/2007/3/section/414 + # 20% x 5,430 = 1,086, matching the Step 5 tax on £5,430. + property_finance_cost_relief: 1_086 income_tax: 0 - name: A landlord within the personal allowance gets no relief and keeps the costs diff --git a/policyengine_uk/tests/test_capital_gains_tax_components.py b/policyengine_uk/tests/test_capital_gains_tax_components.py index 6972af3693..551d358315 100644 --- a/policyengine_uk/tests/test_capital_gains_tax_components.py +++ b/policyengine_uk/tests/test_capital_gains_tax_components.py @@ -40,11 +40,13 @@ def get(name): basic_rate_band_extension = ( get("gift_aid_grossed_up") + personal_pension_band_extension ) + # Gift Aid is no longer in allowances; the pre-Gift-Aid income base + # reproduces the old income/allowance add-backs without double relief. allowances_for_cgt_income = max_( 0, - get("allowances") - get("gift_aid") - personal_pension_band_extension, + get("allowances") - personal_pension_band_extension, ) - taxable_income = max_(0, get("adjusted_net_income") - allowances_for_cgt_income) + taxable_income = max_(0, get("net_income") - allowances_for_cgt_income) gains = max_(0, get("capital_gains")) aea = cgt.annual_exempt_amount gains_less_aea = max_(0, gains - aea) diff --git a/policyengine_uk/tests/test_gift_aid.py b/policyengine_uk/tests/test_gift_aid.py new file mode 100644 index 0000000000..5adfb43b60 --- /dev/null +++ b/policyengine_uk/tests/test_gift_aid.py @@ -0,0 +1,409 @@ +"""Gift Aid invariants and an independent earned-income tax calculation. + +ITA 2007 s.414(2)(a)-(b) treats gifts as net of basic-rate tax and extends +the UK basic/higher limits and the Scottish basic-rate upper and higher +limits. Section 415 defines grossing up; s.58 deducts that gross gift from +adjusted net income, rather than from taxable income: +https://www.legislation.gov.uk/ukpga/2007/3/section/414 +https://www.legislation.gov.uk/ukpga/2007/3/section/415 +https://www.legislation.gov.uk/ukpga/2007/3/section/58 + +The differential integrates the model's existing rate schedules independently. +It preserves their unrelated baseline policy choices, including the Scottish +top-rate threshold, to isolate the effect of extending limits for Gift Aid. +""" + +import numpy as np +import pytest +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st + +from policyengine_uk import Simulation + +pytestmark = pytest.mark.usefixtures("cloned_uk_tax_benefit_system") + +YEAR = 2026 +CASES = 12 +BASE_PEOPLE = 2 * CASES # The same draws, once in rUK and once in Scotland. +PEOPLE = 3 * BASE_PEOPLE # Zero, drawn and larger gifts for each person. +INPUTS = ( + "employment_income", + "savings_interest_income", + "dividend_income", + "property_income", +) +PROPERTY_SETTINGS = settings( + max_examples=30, + deadline=None, + derandomize=True, + suppress_health_check=[HealthCheck.too_slow], +) + +income = st.one_of( + st.integers(0, 300_000), + st.floats(0, 300_000, allow_nan=False, allow_infinity=False, width=32), + st.sampled_from([0, 12_570, 29_526, 43_662, 50_270, 100_000, 101_000, 150_000]), +) +# Multiples of four net pounds give integer gross gifts, avoiding rounding of +# Personal Allowance's statutory whole-pound taper in the random reference. +gift = st.integers(0, 10_000).map(lambda value: value * 4) +earned_cases = st.lists(st.tuples(income, gift, gift), min_size=CASES, max_size=CASES) +mixed_cases = st.lists( + st.tuples(income, income, income, income, gift, gift), + min_size=CASES, + max_size=CASES, +) +basic_cases = st.lists( + st.tuples(st.integers(0, 29_526), gift, gift), + min_size=CASES, + max_size=CASES, +) + + +def situation(rows, gifts=None): + people = { + f"p{i}": { + "age": {YEAR: 40}, + **{name: {YEAR: value} for name, value in zip(INPUTS, row)}, + } + for i, row in enumerate(rows) + } + if gifts is not None: + for i, amount in enumerate(gifts): + people[f"p{i}"]["gift_aid"] = {YEAR: amount} + return { + "people": people, + "benunits": {f"b{i}": {"members": [f"p{i}"]} for i in range(len(rows))}, + "households": { + f"h{i}": { + "members": [f"p{i}"], + "region": { + YEAR: "SCOTLAND" if i % BASE_PEOPLE >= CASES else "NORTH_WEST" + }, + } + for i in range(len(rows)) + }, + } + + +@pytest.fixture(scope="module") +def sim(): + return Simulation(situation=situation([(0, 0, 0, 0)] * PEOPLE, gifts=[0] * PEOPLE)) + + +@pytest.fixture(scope="module") +def omitted_sim(): + return Simulation(situation=situation([(0, 0, 0, 0)] * BASE_PEOPLE)) + + +def run(sim, drawn, *, earned_only=False): + if earned_only: + drawn = [(wages, 0, 0, 0, amount, extra) for wages, amount, extra in drawn] + columns = list(map(np.array, zip(*drawn))) + for name, values in zip(INPUTS, columns[:4]): + # Simulation moves wage inputs to the source of its labour-supply + # formula during initialization. Update that retained input so + # reset_calculations does not discard the new wage draw. + input_name = ( + "employment_income_before_lsr" if name == "employment_income" else name + ) + sim.set_input(input_name, YEAR, np.tile(values, 6)) + amounts, extras = (np.tile(column, 2) for column in columns[-2:]) + intended_gifts = np.concatenate([np.zeros(BASE_PEOPLE), amounts, amounts + extras]) + sim.set_input("gift_aid", YEAR, intended_gifts) + sim.reset_calculations() + np.testing.assert_array_equal(sim.calculate("gift_aid", YEAR), intended_gifts) + for name, values in zip(INPUTS, columns[:4]): + np.testing.assert_array_equal(sim.calculate(name, YEAR), np.tile(values, 6)) + return { + name: sim.calculate(name, YEAR).astype(float) + for name in ( + *INPUTS, + "gift_aid", + "gift_aid_grossed_up", + "personal_allowance", + "adjusted_net_income", + "income_tax", + ) + } + + +def tolerance(*amounts): + """A penny plus rounding of the model's float32 monetary values.""" + return 0.01 + 4e-7 * sum(np.abs(amount) for amount in amounts) + + +def reference_earned_tax(wages, gross_gift, schedule): + """Integrate raised bands directly, without using any model tax formula. + + The Scottish starter/basic boundary stays fixed under s.414(2)(b). + The duplicated UK top rate (an optional reform bracket) has no rate step. + s.58 lowers ANI for the s.35 Personal Allowance taper, not taxable wages. + """ + allowance = max(0, np.ceil(12_570 - max(0, wages - gross_gift - 100_000) / 2)) + taxable = max(0, wages - allowance) + thresholds, rates, eligible_indices = schedule + raised = [ + threshold + (gross_gift if i in eligible_indices else 0) + for i, threshold in enumerate(thresholds) + ] + tax = 0 + for i, (lower, rate) in enumerate(zip(raised, rates)): + upper = raised[i + 1] if i + 1 < len(raised) else float("inf") + tax += rate * max(0, min(taxable, upper) - lower) + return tax + + +def schedules(sim): + rates = sim.tax_benefit_system.parameters(f"{YEAR}-01-01").gov.hmrc.income_tax.rates + return ( + (rates.uk.thresholds, rates.uk.rates, {1, 2}), + ( + rates.scotland.rates.thresholds, + rates.scotland.rates.rates, + set(range(2, len(rates.scotland.rates.thresholds))), + ), + ) + + +@PROPERTY_SETTINGS +@given(earned_cases) +def test_earned_income_matches_independent_raised_limit_reference(sim, drawn): + result = run(sim, drawn, earned_only=True) + scales = schedules(sim) + expected = np.array( + [ + reference_earned_tax(wages, gross_gift, scales[i % BASE_PEOPLE >= CASES]) + for i, (wages, gross_gift) in enumerate( + zip(result["employment_income"], result["gift_aid_grossed_up"]) + ) + ] + ) + assert ( + np.abs(result["income_tax"] - expected) + <= tolerance(expected, result["income_tax"]) + ).all() + + +@PROPERTY_SETTINGS +@given(mixed_cases) +def test_relief_is_never_negative_for_mixed_income(sim, drawn): + result = run(sim, drawn) + baseline, with_gift, _ = np.split(result["income_tax"], 3) + assert (baseline - with_gift >= -tolerance(baseline, with_gift)).all() + + +@PROPERTY_SETTINGS +@given(mixed_cases) +def test_relief_is_non_decreasing_in_gift_for_mixed_income(sim, drawn): + result = run(sim, drawn) + _, with_gift, with_more = np.split(result["income_tax"], 3) + assert (with_more <= with_gift + tolerance(with_gift, with_more)).all() + + +@PROPERTY_SETTINGS +@given(earned_cases) +def test_earned_relief_is_bounded_by_rate_steps_and_allowance_taper(sim, drawn): + result = run(sim, drawn, earned_only=True) + baseline, with_gift, _ = np.split(result["income_tax"], 3) + allowance_before, allowance_after, _ = np.split(result["personal_allowance"], 3) + _, gross_gift, _ = np.split(result["gift_aid_grossed_up"], 3) + scales = schedules(sim) + # All extended thresholds contribute. At £150,000, the UK 20pp and 5pp + # steps save £250 on a £1,000 gross gift, so the largest *single* step + # would incorrectly bound relief at £200. Sum the eligible rate steps. + cumulative_step, top_rate = [], [] + for i in range(BASE_PEOPLE): + _, rates, eligible = scales[i >= CASES] + cumulative_step.append(sum(rates[j] - rates[j - 1] for j in eligible)) + top_rate.append(max(rates)) + limit = gross_gift * np.array(cumulative_step) + ( + allowance_after - allowance_before + ) * np.array(top_rate) + # Earned-only cases isolate this bound from jumps in the Personal Savings + # Allowance when a moved limit changes the model's taxpayer band. + assert (baseline - with_gift <= limit + tolerance(baseline, with_gift)).all() + + +@PROPERTY_SETTINGS +@given(basic_cases) +def test_no_relief_within_unextended_basic_band_below_taper(sim, drawn): + result = run(sim, drawn, earned_only=True) + baseline, with_gift, with_more = np.split(result["income_tax"], 3) + # £29,526 = £12,570 PA + £16,956 Scottish basic upper limit in 2026. + # It is also below rUK's £50,270 basic upper limit and the £100k taper. + np.testing.assert_array_equal(baseline, with_gift) + np.testing.assert_array_equal(baseline, with_more) + + +@PROPERTY_SETTINGS +@given(mixed_cases) +def test_explicit_zero_gift_matches_omitted_input(sim, omitted_sim, drawn): + result = run(sim, drawn) + for name in INPUTS: + input_name = ( + "employment_income_before_lsr" if name == "employment_income" else name + ) + omitted_sim.set_input(input_name, YEAR, result[name][:BASE_PEOPLE]) + omitted_sim.reset_calculations() + for variable in ( + "adjusted_net_income", + "personal_allowance", + "allowances", + "unused_personal_allowance", + "earned_taxable_income", + "earned_income_tax", + "savings_income_tax", + "dividend_income_tax", + "property_income_tax", + "income_tax", + "savings_allowance", + "tax_band", + "capital_gains_tax", + ): + before = omitted_sim.calculate(variable, YEAR) + after = sim.calculate(variable, YEAR)[:BASE_PEOPLE] + assert before.dtype == after.dtype + assert before.tobytes() == after.tobytes(), variable + + +def test_zero_gift_preserves_main_outputs_bit_for_bit(): + """Freeze main's intermediate and final outputs, including mixed income. + + Gift Aid's zero-input compatibility also applies to inherited baseline + choices about property stacking, the Scottish schedule and taxpayer bands. + Exact float32 expectations catch changes hidden by a tax-level tolerance. + """ + rows = [ + (30_000, 0, 0, 0), + (60_000, 0, 0, 0), + (150_000, 0, 0, 0), + (30_000, 12_000, 18_000, 9_000), + (0, 20_000, 10_000, 10_000), + (101_000, 0, 0, 0), + ] * 2 + inputs = situation(rows, gifts=[0] * len(rows)) + for i in range(len(rows)): + inputs["households"][f"h{i}"]["region"][YEAR] = ( + "SCOTLAND" if i >= len(rows) // 2 else "NORTH_WEST" + ) + inputs["people"][f"p{i}"]["capital_gains"] = {YEAR: 40_000} + snapshot = Simulation(situation=inputs) + allowance = [12_570, 12_570, 0, 12_570, 12_570, 12_070] * 2 + expected = { + "adjusted_net_income": [30_000, 60_000, 150_000, 69_000, 40_000, 101_000] * 2, + "personal_allowance": allowance, + "allowances": allowance, + "unused_personal_allowance": [0] * 12, + "earned_taxable_income": [17_430, 47_430, 150_000, 17_430, 0, 88_930] * 2, + "received_allowances_earned_income": [12_570, 12_570, 0, 12_570, 10_000, 12_070] + * 2, + "received_allowances_savings_income": [0, 0, 0, 0, 2_570, 0] * 2, + "received_allowances_dividend_income": [0] * 12, + "basic_rate_earned_income": [17_430, 37_700, 37_700, 17_430, 0, 37_700] * 2, + "higher_rate_earned_income": [0, 9_730, 87_440, 0, 0, 51_230] * 2, + "add_rate_earned_income": [0, 0, 24_860, 0, 0, 0] * 2, + "basic_rate_savings_income": [0, 0, 0, 6_500, 11_430, 0] * 2, + "higher_rate_savings_income": [0] * 12, + "add_rate_savings_income": [0] * 12, + "savings_allowance": [ + 1_000, + 500, + 0, + 500, + 1_000, + 500, + 1_000, + 500, + 0, + 500, + 1_000, + 0, + ], + "earned_income_tax": [ + 3_486, + 11_432, + 53_703, + 3_486, + 0, + 28_032, + 3451.070068359375, + 13182.0498046875, + 60011.44921875, + 3451.070068359375, + 0, + 31407.05078125, + ], + "savings_income_tax": [0, 0, 0, 1_300, 2_286, 0] * 2, + "dividend_income_tax": [0, 0, 0, 4_313.75, 1_021.25, 0] * 2, + "property_income_tax": [0, 0, 0, 1_800, 0, 0, 0, 0, 0, 1_890, 0, 0], + "income_tax": [ + 3_486, + 11_432, + 53_703, + 10_899.75, + 3_307.25, + 28_032, + 3451.070068359375, + 13182.0498046875, + 60011.44921875, + 10954.8203125, + 3_307.25, + 31407.05078125, + ], + "capital_gains_tax": [ + 7663.7998046875, + 8_880, + 8_880, + 8_880, + 8263.7998046875, + 8_880, + ] + * 2, + } + for variable, values in expected.items(): + actual = snapshot.calculate(variable, YEAR) + assert actual.tobytes() == np.array(values, dtype=actual.dtype).tobytes(), ( + variable + ) + assert snapshot.calculate("tax_band", YEAR).tolist() == [ + "BASIC", + "HIGHER", + "ADDITIONAL", + "HIGHER", + "BASIC", + "HIGHER", + "INTERMEDIATE", + "HIGHER", + "ADDITIONAL", + "HIGHER", + "INTERMEDIATE", + "ADDITIONAL", + ] + + +@pytest.mark.parametrize("ani,net_gift", [(50_000, 0), (49_000, 800)]) +def test_explicit_adjusted_net_income_preserves_tax_base(ani, net_gift): + """An input ANI already includes the s.58 gross Gift Aid adjustment. + + Reconstruct net income once when ANI is supplied directly. Zero Gift Aid + must retain the longstanding ability to supply this intermediate input. + """ + supplied_inputs = situation([(0, 0, 0, 0)], gifts=[net_gift]) + supplied_inputs["people"]["p0"]["adjusted_net_income"] = {YEAR: ani} + supplied = Simulation(situation=supplied_inputs) + raw = Simulation(situation=situation([(50_000, 0, 0, 0)], gifts=[net_gift])) + for variable in ( + "net_income", + "adjusted_net_income", + "personal_allowance", + "earned_taxable_income", + "income_tax", + ): + before = raw.calculate(variable, YEAR) + after = supplied.calculate(variable, YEAR) + assert before.dtype == after.dtype + assert before.tobytes() == after.tobytes(), variable + assert supplied.calculate("net_income", YEAR)[0] == 50_000 + assert supplied.calculate("earned_taxable_income", YEAR)[0] == 37_430 + assert supplied.calculate("income_tax", YEAR)[0] == 7_486 diff --git a/policyengine_uk/utils/income_tax.py b/policyengine_uk/utils/income_tax.py new file mode 100644 index 0000000000..23201e2fd6 --- /dev/null +++ b/policyengine_uk/utils/income_tax.py @@ -0,0 +1,29 @@ +import numpy as np + + +def calc_income_tax_with_gift_aid( + scale, income, gross_gift, first_extended_threshold=1, last_extended_threshold=3 +): + """Apply ITA 2007 s.414(2)(b) to a per-person marginal-rate schedule. + + Moving a threshold t to t + G removes the rate step on the overlap + with [t, t + G]. Summing those steps remains exact even when a gift + spans several bands. Starting at index 2 leaves the Scottish starter + band's upper limit unchanged. The default stop extends only the UK + basic and higher limits, excluding the model's optional extra bracket. + Scottish callers extend all subsequent limits. The scale is never mutated. + https://www.legislation.gov.uk/ukpga/2007/3/section/414 + """ + tax = scale.calc(income) + for index in range( + first_extended_threshold, min(last_extended_threshold, len(scale.thresholds)) + ): + overlap = np.clip(income - scale.thresholds[index], 0, gross_gift) + tax = tax - (scale.rates[index] - scale.rates[index - 1]) * overlap + return tax + + +def first_extended_scottish_threshold(scale): + # The three-rate schedule before 2018 has no starter rate. Later + # schedules have starter, basic, intermediate and higher rates. + return 1 if len(scale.thresholds) == 3 else 2 diff --git a/policyengine_uk/utils/supplied_inputs.py b/policyengine_uk/utils/supplied_inputs.py index 3e7d34caf7..d2bfdfb77c 100644 --- a/policyengine_uk/utils/supplied_inputs.py +++ b/policyengine_uk/utils/supplied_inputs.py @@ -26,6 +26,7 @@ # Variables that formulas ask about through the helpers below. SUPPLIED_INPUT_VARIABLES = frozenset( { + "adjusted_net_income", "bus_fare_spending", "bus_in_london_trips", "other_local_bus_trips", diff --git a/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_candidate_benunit_income.py b/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_candidate_benunit_income.py index 1ea120174b..9409309222 100644 --- a/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_candidate_benunit_income.py +++ b/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_candidate_benunit_income.py @@ -13,4 +13,4 @@ class maintenance_loan_candidate_benunit_income(Variable): unit = GBP def formula(person, period, parameters): - return person.benunit.sum(person.benunit.members("adjusted_net_income", period)) + return person.benunit.sum(person.benunit.members("net_income", period)) diff --git a/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_household_income.py b/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_household_income.py index 4aaf08b03e..816c2e2296 100644 --- a/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_household_income.py +++ b/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_household_income.py @@ -42,7 +42,7 @@ def formula(person, period, parameters): else np.zeros_like(has_sponsor, dtype=bool) ) benunit_income = person.benunit.sum( - person.benunit.members("adjusted_net_income", period) + person.benunit.members("net_income", period) ) dependent_student_proxy = ( in_higher_education diff --git a/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_income_tax_on_earnings.py b/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_income_tax_on_earnings.py index 88ba53027c..bd3d93a2ff 100644 --- a/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_income_tax_on_earnings.py +++ b/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_income_tax_on_earnings.py @@ -1,4 +1,8 @@ from policyengine_uk.model_api import * +from policyengine_uk.utils.income_tax import ( + calc_income_tax_with_gift_aid, + first_extended_scottish_threshold, +) class uc_income_tax_on_earnings(Variable): @@ -44,7 +48,7 @@ def formula(person, period, parameters): # Pension coded against it; that is not modelled. p = parameters(period) income_tax = p.gov.hmrc.income_tax - # earned_taxable_income is adjusted net income less the incomes and + # earned_taxable_income is net income less the incomes and # allowances on the exclusions list. Read that list, so a reform # that changes it (for example one that exempts pensions) is # followed here. @@ -74,7 +78,7 @@ def formula(person, period, parameters): ) # The income left in earned_taxable_income before allowances. The # part of it above earnings is the person's other non-savings income. - income_in_base = person("adjusted_net_income", period) - add( + income_in_base = person("net_income", period) - add( person, period, excluded_incomes ) other_income = max_(0, income_in_base - earnings) @@ -82,10 +86,17 @@ def formula(person, period, parameters): 0, person("earned_taxable_income", period) - other_income ) rates = income_tax.rates + gross_gift = person("gift_aid_grossed_up", period) tax = where( person("pays_scottish_income_tax", period), - rates.scotland.rates.calc(taxable_earnings), - rates.uk.calc(taxable_earnings), + calc_income_tax_with_gift_aid( + rates.scotland.rates, + taxable_earnings, + gross_gift, + first_extended_scottish_threshold(rates.scotland.rates), + len(rates.scotland.rates.thresholds), + ), + calc_income_tax_with_gift_aid(rates.uk, taxable_earnings, gross_gift), ) # Tax reductions (for example the married couple's allowance) come # off the tax on earnings first, as allowances do. So the deduction diff --git a/policyengine_uk/variables/gov/hmrc/capital_gains_tax/capital_gains_tax.py b/policyengine_uk/variables/gov/hmrc/capital_gains_tax/capital_gains_tax.py index 9a9fef5e8e..31a414c0fb 100644 --- a/policyengine_uk/variables/gov/hmrc/capital_gains_tax/capital_gains_tax.py +++ b/policyengine_uk/variables/gov/hmrc/capital_gains_tax/capital_gains_tax.py @@ -56,13 +56,11 @@ def formula(person, period, parameters): ) allowances_for_cgt_income = max_( 0, - person("allowances", period) - - person("gift_aid", period) - - personal_pension_band_extension, + person("allowances", period) - personal_pension_band_extension, ) taxable_income = max_( 0, - person("adjusted_net_income", period) - allowances_for_cgt_income, + person("net_income", period) - allowances_for_cgt_income, ) capital_gains = person("capital_gains", period) gains = max_(0, capital_gains) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/adjusted_net_income.py b/policyengine_uk/variables/gov/hmrc/income_tax/adjusted_net_income.py index 3723b6a60a..dc62518612 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/adjusted_net_income.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/adjusted_net_income.py @@ -4,27 +4,15 @@ class adjusted_net_income(Variable): value_type = float entity = Person - label = "Taxable income after tax reliefs and before allowances" + label = "Adjusted net income" definition_period = YEAR reference = dict( - title="Income Tax Act 2007, s. 23", - href="https://www.legislation.gov.uk/ukpga/2007/3/section/23", + title="Income Tax Act 2007, s. 58", + href="https://www.legislation.gov.uk/ukpga/2007/3/section/58", ) unit = GBP def formula(person, period, parameters): - adjusted_net_income_components = parameters( - period - ).gov.hmrc.income_tax.adjusted_net_income_components - - # Find adjusted net income - ani = add(person, period, adjusted_net_income_components) - - # For basic income contributions, add basic income - # Modifying param list directly is mutative, hence two-step process - if parameters( - period - ).gov.contrib.ubi_center.basic_income.interactions.include_in_taxable_income: - ani += person("basic_income", period) - - return max_(0, ani) + # ITA 2007 s.58, Step 2: deduct the gift grossed up at the basic rate. + # This adjustment affects the PA taper and HICBC, not taxable income. + return person("net_income", period) - person("gift_aid_grossed_up", period) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/allowances.py b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/allowances.py index 1225fdc7f2..229589a4fc 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/allowances.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/allowances.py @@ -8,10 +8,11 @@ class allowances(Variable): definition_period = YEAR unit = GBP + # ITA 2007 s.414(2): Gift Aid extends rate limits rather than deducting + # the gift from the income charged to tax. adds = [ "personal_allowance", "blind_persons_allowance", - "gift_aid", "covenanted_payments", "charitable_investment_gifts", "other_deductions", diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/gift_aid.py b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/gift_aid.py index 58ead1962c..c814ae0be0 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/gift_aid.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/gift_aid.py @@ -20,8 +20,8 @@ class gift_aid_grossed_up(Variable): definition_period = YEAR unit = GBP reference = dict( - title="Income Tax Act 2007, s. 58 (2)", - href="https://www.legislation.gov.uk/ukpga/2007/3/section/58", + title="Income Tax Act 2007, ss. 414(2)(a) and 415", + href="https://www.legislation.gov.uk/ukpga/2007/3/section/415", ) def formula(person, period, parameters): diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/pension_annual_allowance.py b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/pension_annual_allowance.py index c066e301c1..a939105d82 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/pension_annual_allowance.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/pension_annual_allowance.py @@ -10,6 +10,6 @@ class pension_annual_allowance(Variable): def formula(person, period, parameters): allowance = parameters(period).gov.hmrc.income_tax.allowances.annual_allowance - ANI = person("adjusted_net_income", period) + ANI = person("net_income", period) reduction = max_(0, ANI - allowance.taper) * allowance.reduction_rate return max_(allowance.minimum, allowance.default - reduction) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/personal_allowance.py b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/personal_allowance.py index 52efd2fab7..735684812c 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/personal_allowance.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/personal_allowance.py @@ -17,10 +17,7 @@ def formula(person, period, parameters): PA = params.gov.hmrc.income_tax.allowances.personal_allowance personal_allowance = PA.amount ANI = person("adjusted_net_income", period) - # Per ITA 2007 s.58, deduct grossed-up Gift Aid from ANI - # when calculating Personal Allowance taper - gift_aid_grossed_up = person("gift_aid_grossed_up", period) - ANI_for_taper = ANI - gift_aid_grossed_up - excess = max_(0, ANI_for_taper - PA.maximum_ANI) + # adjusted_net_income already includes the s.58 Gift Aid adjustment. + excess = max_(0, ANI - PA.maximum_ANI) reduction = excess * PA.reduction_rate return max_(0, np.ceil(personal_allowance - reduction)) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/received_allowances/received_allowances_earned_income.py b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/received_allowances/received_allowances_earned_income.py index bcdd36dcd1..3103af399a 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/received_allowances/received_allowances_earned_income.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/received_allowances/received_allowances_earned_income.py @@ -14,7 +14,7 @@ def formula(person, period, parameters): all_allowances = person("allowances", period) - all_income = person("adjusted_net_income", period) + all_income = person("net_income", period) excluded_income = person("taxable_savings_interest_income", period) + person( "taxable_dividend_income", period ) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/unused_personal_allowance.py b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/unused_personal_allowance.py index 4257638e61..8a289e9aaf 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/allowances/unused_personal_allowance.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/allowances/unused_personal_allowance.py @@ -11,7 +11,6 @@ class unused_personal_allowance(Variable): def formula(person, period, parameters): return max_( - person("personal_allowance", period) - - person("adjusted_net_income", period), + person("personal_allowance", period) - person("net_income", period), 0, ) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/add_rate_earned_income.py b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/add_rate_earned_income.py index 73ba9eaaa0..eb2ea3f511 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/add_rate_earned_income.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/add_rate_earned_income.py @@ -15,4 +15,8 @@ class add_rate_earned_income(Variable): def formula(person, period, parameters): income = person("earned_taxable_income", period) thresholds = parameters(period).gov.hmrc.income_tax.rates.uk.thresholds - return clip(income, thresholds[2], inf) - thresholds[2] + # ITA 2007 s.414(2)(b): extend both UK rate limits by the gross gift. + gross_gift = person("gift_aid_grossed_up", period) + return clip(income, (thresholds[2] + gross_gift), inf) - ( + thresholds[2] + gross_gift + ) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/basic_rate_earned_income.py b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/basic_rate_earned_income.py index 7fdf1c463f..c620aaef59 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/basic_rate_earned_income.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/basic_rate_earned_income.py @@ -15,4 +15,6 @@ class basic_rate_earned_income(Variable): def formula(person, period, parameters): income = person("earned_taxable_income", period) thresholds = parameters(period).gov.hmrc.income_tax.rates.uk.thresholds - return clip(income, thresholds[0], thresholds[1]) + # ITA 2007 s.414(2)(b): extend both UK rate limits by the gross gift. + gross_gift = person("gift_aid_grossed_up", period) + return clip(income, thresholds[0], (thresholds[1] + gross_gift)) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/earned_taxable_income.py b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/earned_taxable_income.py index 98dba2b510..5de4387597 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/earned_taxable_income.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/earned_taxable_income.py @@ -16,6 +16,6 @@ def formula(person, period, parameters): exclusion_list = parameters( period ).gov.hmrc.income_tax.earned_taxable_income_exclusions - ani = person("adjusted_net_income", period) + ani = person("net_income", period) exclusions = add(person, period, exclusion_list) return max_(0, ani - exclusions) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/higher_rate_earned_income.py b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/higher_rate_earned_income.py index 6ea178bb96..813a231312 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/higher_rate_earned_income.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_earned_income/higher_rate_earned_income.py @@ -15,4 +15,8 @@ class higher_rate_earned_income(Variable): def formula(person, period, parameters): income = person("earned_taxable_income", period) thresholds = parameters(period).gov.hmrc.income_tax.rates.uk.thresholds - return clip(income, thresholds[1], thresholds[2]) - thresholds[1] + # ITA 2007 s.414(2)(b): extend both UK rate limits by the gross gift. + gross_gift = person("gift_aid_grossed_up", period) + return clip( + income, (thresholds[1] + gross_gift), (thresholds[2] + gross_gift) + ) - (thresholds[1] + gross_gift) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_liability/tax_band.py b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_liability/tax_band.py index 7a67966c9f..62f4794ba6 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_liability/tax_band.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_liability/tax_band.py @@ -1,4 +1,5 @@ from policyengine_uk.model_api import * +from policyengine_uk.utils.income_tax import first_extended_scottish_threshold class TaxBand(Enum): @@ -24,11 +25,16 @@ class tax_band(Variable): def formula(person, period, parameters): allowances = person("allowances", period) - ani = person("adjusted_net_income", period) + ani = person("net_income", period) rates = parameters(period).gov.hmrc.income_tax.rates - basic = allowances + rates.uk.thresholds[0] - higher = allowances + rates.uk.thresholds[-2] - add = allowances + rates.uk.thresholds[-1] + gross_gift = person("gift_aid_grossed_up", period) + limits = [ + threshold + (gross_gift if index in (1, 2) else 0) + for index, threshold in enumerate(rates.uk.thresholds) + ] + basic = allowances + limits[0] + higher = allowances + limits[-2] + add = allowances + limits[-1] return select( [ani >= add, ani >= higher, ani > basic], [TaxBand.ADDITIONAL, TaxBand.HIGHER, TaxBand.BASIC], @@ -37,17 +43,26 @@ def formula(person, period, parameters): def formula_2017_04_06(person, period, parameters): allowances = person("allowances", period) - ani = person("adjusted_net_income", period) + ani = person("net_income", period) rates = parameters(period).gov.hmrc.income_tax.rates scot = person("pays_scottish_income_tax", period) income = ani - allowances + gross_gift = person("gift_aid_grossed_up", period) + # s.414(2)(b) extends the limits used to determine the tax band. uk_band = select( - [income < threshold for threshold in rates.uk.thresholds[:3]], + [ + income < threshold + (gross_gift if index > 0 else 0) + for index, threshold in enumerate(rates.uk.thresholds[:3]) + ], [TaxBand.NONE, TaxBand.BASIC, TaxBand.HIGHER], default=TaxBand.ADDITIONAL, ) + first_extended = first_extended_scottish_threshold(rates.scotland.rates) scottish_band = select( - [income < threshold for threshold in rates.scotland.rates.thresholds[:3]], + [ + income < threshold + (gross_gift if index >= first_extended else 0) + for index, threshold in enumerate(rates.scotland.rates.thresholds[:3]) + ], [TaxBand.NONE, TaxBand.BASIC, TaxBand.HIGHER], default=TaxBand.ADDITIONAL, ) @@ -55,17 +70,25 @@ def formula_2017_04_06(person, period, parameters): def formula_2018_06_01(person, period, parameters): allowances = person("allowances", period) - ani = person("adjusted_net_income", period) + ani = person("net_income", period) rates = parameters(period).gov.hmrc.income_tax.rates scot = person("pays_scottish_income_tax", period) income = ani - allowances + gross_gift = person("gift_aid_grossed_up", period) + # s.414(2)(b) extends the limits used to determine the tax band. uk_band = select( - [income < threshold for threshold in rates.uk.thresholds[:3]], + [ + income < threshold + (gross_gift if index > 0 else 0) + for index, threshold in enumerate(rates.uk.thresholds[:3]) + ], [TaxBand.NONE, TaxBand.BASIC, TaxBand.HIGHER], default=TaxBand.ADDITIONAL, ) scottish_band = select( - [income < threshold for threshold in rates.scotland.rates.thresholds[:5]], + [ + income < threshold + (gross_gift if index >= 2 else 0) + for index, threshold in enumerate(rates.scotland.rates.thresholds[:5]) + ], [ TaxBand.NONE, TaxBand.STARTER, diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/add_rate_savings_income.py b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/add_rate_savings_income.py index 610b9481b2..6e09890aeb 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/add_rate_savings_income.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/add_rate_savings_income.py @@ -14,6 +14,8 @@ class add_rate_savings_income(Variable): def formula(person, period, parameters): thresholds = parameters(period).gov.hmrc.income_tax.rates.uk.thresholds + # ITA 2007 s.414(2)(b): extend both UK rate limits by the gross gift. + gross_gift = person("gift_aid_grossed_up", period) other_income = person("earned_taxable_income", period) savings_after_allowances = max_( 0, @@ -28,8 +30,10 @@ def formula(person, period, parameters): taxable_savings_start = other_income + zero_rate_savings add_rate_amount_with = clip( other_income + savings_after_allowances, - thresholds[2], + (thresholds[2] + gross_gift), inf, ) - add_rate_amount_without = clip(taxable_savings_start, thresholds[2], inf) + add_rate_amount_without = clip( + taxable_savings_start, (thresholds[2] + gross_gift), inf + ) return max_(0, add_rate_amount_with - add_rate_amount_without) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/basic_rate_savings_income.py b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/basic_rate_savings_income.py index cea212f46d..137055efcb 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/basic_rate_savings_income.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/basic_rate_savings_income.py @@ -14,6 +14,8 @@ class basic_rate_savings_income(Variable): def formula(person, period, parameters): thresholds = parameters(period).gov.hmrc.income_tax.rates.uk.thresholds + # ITA 2007 s.414(2)(b): extend both UK rate limits by the gross gift. + gross_gift = person("gift_aid_grossed_up", period) other_income = person("earned_taxable_income", period) savings_after_allowances = max_( 0, @@ -29,11 +31,11 @@ def formula(person, period, parameters): basic_rate_amount_with = clip( other_income + savings_after_allowances, thresholds[0], - thresholds[1], + (thresholds[1] + gross_gift), ) basic_rate_amount_without = clip( taxable_savings_start, thresholds[0], - thresholds[1], + (thresholds[1] + gross_gift), ) return max_(0, basic_rate_amount_with - basic_rate_amount_without) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/basic_rate_savings_income_pre_starter.py b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/basic_rate_savings_income_pre_starter.py index a3f99e40ce..544671c0e7 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/basic_rate_savings_income_pre_starter.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/basic_rate_savings_income_pre_starter.py @@ -14,6 +14,8 @@ class basic_rate_savings_income_pre_starter(Variable): def formula(person, period, parameters): thresholds = parameters(period).gov.hmrc.income_tax.rates.uk.thresholds + # ITA 2007 s.414(2)(b): extend both UK rate limits by the gross gift. + gross_gift = person("gift_aid_grossed_up", period) savings_income = max_( 0, person("taxable_savings_interest_income", period) @@ -23,9 +25,9 @@ def formula(person, period, parameters): basic_rate_amount_with_savings = clip( savings_income + other_income, thresholds[0], - thresholds[1], + (thresholds[1] + gross_gift), ) basic_rate_amount_without_savings = clip( - other_income, thresholds[0], thresholds[1] + other_income, thresholds[0], (thresholds[1] + gross_gift) ) return basic_rate_amount_with_savings - basic_rate_amount_without_savings diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/higher_rate_savings_income.py b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/higher_rate_savings_income.py index cfb7da1a31..4ff3f734cb 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/higher_rate_savings_income.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/bracketized_savings_income/higher_rate_savings_income.py @@ -14,6 +14,8 @@ class higher_rate_savings_income(Variable): def formula(person, period, parameters): thresholds = parameters(period).gov.hmrc.income_tax.rates.uk.thresholds + # ITA 2007 s.414(2)(b): extend both UK rate limits by the gross gift. + gross_gift = person("gift_aid_grossed_up", period) other_income = person("earned_taxable_income", period) savings_after_allowances = max_( 0, @@ -28,12 +30,12 @@ def formula(person, period, parameters): taxable_savings_start = other_income + zero_rate_savings higher_rate_amount_with = clip( other_income + savings_after_allowances, - thresholds[1], - thresholds[2], + (thresholds[1] + gross_gift), + (thresholds[2] + gross_gift), ) higher_rate_amount_without = clip( taxable_savings_start, - thresholds[1], - thresholds[2], + (thresholds[1] + gross_gift), + (thresholds[2] + gross_gift), ) return max_(0, higher_rate_amount_with - higher_rate_amount_without) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/earned_income_tax.py b/policyengine_uk/variables/gov/hmrc/income_tax/earned_income_tax.py index b3fb1187bf..30f71c00ed 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/earned_income_tax.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/earned_income_tax.py @@ -1,4 +1,8 @@ from policyengine_uk.model_api import * +from policyengine_uk.utils.income_tax import ( + calc_income_tax_with_gift_aid, + first_extended_scottish_threshold, +) class earned_income_tax(Variable): @@ -14,9 +18,16 @@ class earned_income_tax(Variable): def formula(person, period, parameters): rates = parameters(period).gov.hmrc.income_tax.rates - + income = person("earned_taxable_income", period) + gross_gift = person("gift_aid_grossed_up", period) return where( person("pays_scottish_income_tax", period), - rates.scotland.rates.calc(person("earned_taxable_income", period)), - rates.uk.calc(person("earned_taxable_income", period)), + calc_income_tax_with_gift_aid( + rates.scotland.rates, + income, + gross_gift, + first_extended_scottish_threshold(rates.scotland.rates), + len(rates.scotland.rates.thresholds), + ), + calc_income_tax_with_gift_aid(rates.uk, income, gross_gift), ) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/liability/dividend_income_tax.py b/policyengine_uk/variables/gov/hmrc/income_tax/liability/dividend_income_tax.py index e812a8f769..8a063c2652 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/liability/dividend_income_tax.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/liability/dividend_income_tax.py @@ -1,4 +1,5 @@ from policyengine_uk.model_api import * +from policyengine_uk.utils.income_tax import calc_income_tax_with_gift_aid class dividend_income_tax(Variable): @@ -28,10 +29,11 @@ def formula(person, period, parameters): parameters(period).gov.hmrc.income_tax.allowances.dividend_allowance, dividends_after_allowances, ) - tax_with_dividends = rates.dividends.calc( - other_income + dividends_after_allowances + gross_gift = person("gift_aid_grossed_up", period) + tax_with_dividends = calc_income_tax_with_gift_aid( + rates.dividends, other_income + dividends_after_allowances, gross_gift ) - tax_with_dividend_allowance = rates.dividends.calc( - other_income + dividend_allowance + tax_with_dividend_allowance = calc_income_tax_with_gift_aid( + rates.dividends, other_income + dividend_allowance, gross_gift ) return max_(0, tax_with_dividends - tax_with_dividend_allowance) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/liability/property_income_tax.py b/policyengine_uk/variables/gov/hmrc/income_tax/liability/property_income_tax.py index 0fff929eac..f9bc3374da 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/liability/property_income_tax.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/liability/property_income_tax.py @@ -1,4 +1,8 @@ from policyengine_uk.model_api import * +from policyengine_uk.utils.income_tax import ( + calc_income_tax_with_gift_aid, + first_extended_scottish_threshold, +) class property_income_tax(Variable): @@ -37,7 +41,7 @@ def formula(person, period, parameters): taxable_property = person("taxable_property_income", period) non_savings_after_allowances = max_( 0, - person("adjusted_net_income", period) + person("net_income", period) - person("taxable_savings_interest_income", period) - person("taxable_dividend_income", period) - person("allowances", period), @@ -50,15 +54,27 @@ def formula(person, period, parameters): # Scottish taxpayers: use Scottish rates via calc method # Tax on (other earned + property) minus tax on (other earned) - scottish_tax = rates.scotland.rates.calc( - other_earned_taxable + property_after_allowances - ) - rates.scotland.rates.calc(other_earned_taxable) + gross_gift = person("gift_aid_grossed_up", period) + first_extended = first_extended_scottish_threshold(rates.scotland.rates) + scottish_tax = calc_income_tax_with_gift_aid( + rates.scotland.rates, + other_earned_taxable + property_after_allowances, + gross_gift, + first_extended, + len(rates.scotland.rates.thresholds), + ) - calc_income_tax_with_gift_aid( + rates.scotland.rates, + other_earned_taxable, + gross_gift, + first_extended, + len(rates.scotland.rates.thresholds), + ) # rUK taxpayers: use property-specific rates property_rates = rates.property thresholds = rates.uk.thresholds - basic_upper = thresholds[1] # Higher rate threshold - higher_upper = thresholds[2] # Additional rate threshold + basic_upper = thresholds[1] + gross_gift + higher_upper = thresholds[2] + gross_gift # Property income in basic rate band basic_rate_space = max_(0, basic_upper - other_earned_taxable) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/net_income.py b/policyengine_uk/variables/gov/hmrc/income_tax/net_income.py new file mode 100644 index 0000000000..d10575ae77 --- /dev/null +++ b/policyengine_uk/variables/gov/hmrc/income_tax/net_income.py @@ -0,0 +1,37 @@ +from policyengine_uk.model_api import * +from policyengine_uk.utils.supplied_inputs import supplied_input + + +class net_income(Variable): + value_type = float + entity = Person + label = "Income before personal allowances and Gift Aid adjustments" + documentation = ( + "The model's taxable income components before the Gift Aid adjustment " + "to adjusted net income. Gift Aid extends rate limits and reduces " + "adjusted net income, but does not reduce the income charged to tax." + ) + definition_period = YEAR + reference = dict( + title="Income Tax Act 2007, ss. 23 and 58", + href="https://www.legislation.gov.uk/ukpga/2007/3/section/58", + ) + unit = GBP + + def formula(person, period, parameters): + # An explicitly supplied ANI already includes the s.58 adjustment. + # Restore it for the taxable base, preserving direct ANI inputs at + # zero Gift Aid. Ordinary cases compute the base directly to avoid + # subtracting and re-adding a large gift through a float32 ANI cache. + supplied_ani = supplied_input(person, "adjusted_net_income", period) + if supplied_ani is not None: + return supplied_ani + person("gift_aid_grossed_up", period) + components = parameters( + period + ).gov.hmrc.income_tax.adjusted_net_income_components + income = add(person, period, components) + if parameters( + period + ).gov.contrib.ubi_center.basic_income.interactions.include_in_taxable_income: + income += person("basic_income", period) + return max_(0, income) diff --git a/policyengine_uk/variables/gov/hmrc/income_tax/reliefs/property_finance_costs_relieved.py b/policyengine_uk/variables/gov/hmrc/income_tax/reliefs/property_finance_costs_relieved.py index 1ca09de9fc..b0553c1c2c 100644 --- a/policyengine_uk/variables/gov/hmrc/income_tax/reliefs/property_finance_costs_relieved.py +++ b/policyengine_uk/variables/gov/hmrc/income_tax/reliefs/property_finance_costs_relieved.py @@ -46,7 +46,7 @@ def formula(person, period, parameters): ) adjusted_total_income = max_( 0, - person("adjusted_net_income", period) + person("net_income", period) - person("taxable_savings_interest_income", period) - person("taxable_dividend_income", period) - step_2_reliefs diff --git a/policyengine_uk/variables/gov/hmrc/student_loans/plan_2_interest_rate.py b/policyengine_uk/variables/gov/hmrc/student_loans/plan_2_interest_rate.py index fe52b5f26a..ec5a2f2b5e 100644 --- a/policyengine_uk/variables/gov/hmrc/student_loans/plan_2_interest_rate.py +++ b/policyengine_uk/variables/gov/hmrc/student_loans/plan_2_interest_rate.py @@ -16,7 +16,7 @@ class plan_2_interest_rate(Variable): reference = "https://www.legislation.gov.uk/uksi/2012/1309/regulation/10" def formula(person, period, parameters): - income = person("adjusted_net_income", period) + income = person("net_income", period) p = parameters(period).gov.hmrc.student_loans rpi = parameters(period).gov.economic_assumptions.yoy_growth.obr.rpi diff --git a/policyengine_uk/variables/gov/hmrc/student_loans/student_loan_repayment.py b/policyengine_uk/variables/gov/hmrc/student_loans/student_loan_repayment.py index a7590eb33a..f086a65126 100644 --- a/policyengine_uk/variables/gov/hmrc/student_loans/student_loan_repayment.py +++ b/policyengine_uk/variables/gov/hmrc/student_loans/student_loan_repayment.py @@ -19,7 +19,7 @@ class student_loan_repayment(Variable): def formula(person, period, parameters): plan = person("student_loan_plan", period) - income = person("adjusted_net_income", period) + income = person("net_income", period) balance = max_(0, person("student_loan_balance", period)) p = parameters(period).gov.hmrc.student_loans From 64dff7b228ac547ae5b83565194dfeec31d8ee3a Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 09:24:00 -0400 Subject: [PATCH 02/27] Keep a supplied adjusted net income in the marriage-neutral reform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the Gift Aid fix. The reform's net_income override read the income components only, so a supplied adjusted_net_income no longer set the taxable base: a married couple with supplied ANI of £50,000 and £0 paid no tax instead of £7,486. The override now returns a supplied ANI plus the grossed-up gift, as the baseline net_income does. Also from the review: - a helper test against each scale shape callers pass: a UK extra bracket at a distinct rate, and the three- and six-rate Scottish scales; - 2017-18 Scottish YAML cases on the three-rate scale; - a frozen positive-gift CGT case (unchanged from main); - corrected descriptions of the CGT test oracle and the maintenance loan income measure. Co-Authored-By: Claude Opus 5.5 --- .../reforms/cps/marriage_tax_reforms.py | 8 ++ .../gov/hmrc/income_tax/gift_aid.yaml | 93 +++++++++++++++++++ .../reforms/cps/marriage_tax_reforms.yaml | 66 +++++++++++++ .../test_capital_gains_tax_components.py | 7 +- policyengine_uk/tests/test_gift_aid.py | 69 ++++++++++++++ .../maintenance_loan_household_income.py | 2 +- 6 files changed, 242 insertions(+), 3 deletions(-) diff --git a/policyengine_uk/reforms/cps/marriage_tax_reforms.py b/policyengine_uk/reforms/cps/marriage_tax_reforms.py index 73efa66e79..32cedd3f82 100644 --- a/policyengine_uk/reforms/cps/marriage_tax_reforms.py +++ b/policyengine_uk/reforms/cps/marriage_tax_reforms.py @@ -1,4 +1,5 @@ from policyengine_uk.model_api import * +from policyengine_uk.utils.supplied_inputs import supplied_input from typing import Union, Optional @@ -163,6 +164,13 @@ class net_income(Variable): value_type = float def formula(person, period, parameters): + # A supplied adjusted net income already includes the s.58 Gift + # Aid step, and it took precedence over this reform's split before + # the reform moved to net_income. Keep that: restore the gift for + # the taxable base, as the baseline net_income does. + supplied_ani = supplied_input(person, "adjusted_net_income", period) + if supplied_ani is not None: + return supplied_ani + person("gift_aid_grossed_up", period) income = person("unadjusted_net_income", period) # Split between the spouses or civil partners only, never their # children. diff --git a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml index 43124e502e..e3ca92829f 100644 --- a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml +++ b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml @@ -302,3 +302,96 @@ adjusted_net_income: [50_000, 50_000] earned_taxable_income: [37_430, 37_430] income_tax: [7_486, 7_486] + +# Before 2018-19 the Scottish scale had three rates (basic, higher, additional), +# so s.414(2)(b) raises its first limit, not the second. +- name: Gift Aid raises the three-rate Scottish basic rate limit in 2017-18 + period: 2017 + absolute_error_margin: 0.01 + input: + people: + donor: + age: 40 + employment_income: 60_000 + gift_aid: 800 + control: + age: 40 + employment_income: 60_000 + benunits: + donor_unit: + members: [donor] + control_unit: + members: [control] + households: + donor_household: + members: [donor] + region: SCOTLAND + control_household: + members: [control] + region: SCOTLAND + output: + # Taxable £48,500 with a £31,500 basic rate limit; the gift raises it to + # £32,500, moving £1,000 from 40% to 20%. + income_tax: [12_900, 13_100] + +- name: Gift Aid raises the three-rate Scottish higher rate limit in 2017-18 + period: 2017 + absolute_error_margin: 0.01 + input: + people: + donor: + age: 40 + employment_income: 160_000 + gift_aid: 800 + control: + age: 40 + employment_income: 160_000 + benunits: + donor_unit: + members: [donor] + control_unit: + members: [control] + households: + donor_household: + members: [donor] + region: SCOTLAND + control_household: + members: [control] + region: SCOTLAND + output: + # No personal allowance either way. £1,000 moves from 40% to 20% (£200) + # and £1,000 from 45% to 40% (£50). + income_tax: [57_950, 58_200] + +- name: Gift Aid leaves capital gains tax as it was, with its band extension + period: 2026 + absolute_error_margin: 0.01 + input: + people: + donor: + age: 40 + employment_income: 45_000 + capital_gains: 20_000 + gift_aid: 800 + control: + age: 40 + employment_income: 45_000 + capital_gains: 20_000 + benunits: + donor_unit: + members: [donor] + control_unit: + members: [control] + households: + donor_household: + members: [donor] + region: NORTH_WEST + control_household: + members: [control] + region: NORTH_WEST + output: + # Frozen from main a3752304e, which gave the same CGT: £17,000 taxable + # gains above the £3,000 exempt amount. With the gift, £6,270 of basic + # band remains (£5,270 without), at 18%, and the rest at 24%. + capital_gains_tax: [3_703.80, 3_763.80] + income_tax: [6_486, 6_486] diff --git a/policyengine_uk/tests/policy/reforms/cps/marriage_tax_reforms.yaml b/policyengine_uk/tests/policy/reforms/cps/marriage_tax_reforms.yaml index c557150fb9..7dd1c38ab0 100644 --- a/policyengine_uk/tests/policy/reforms/cps/marriage_tax_reforms.yaml +++ b/policyengine_uk/tests/policy/reforms/cps/marriage_tax_reforms.yaml @@ -185,3 +185,69 @@ members: [high_earner, low_earner, student] output: adjusted_net_income: [35_000, 35_000, 8_000] + +# A supplied adjusted net income took precedence over this reform before +# #2253 moved its override to net_income; it still sets the taxable base. +- name: Marriage-neutral IT — a supplied adjusted net income still sets the taxable base + period: 2026 + absolute_error_margin: 1 + reforms: + - policyengine_uk.reforms.cps.marriage_tax_reforms.marriage_neutral_it_reform + input: + people: + earner: + age: 40 + adjusted_net_income: 50_000 + personal_allowance: 12_570 + marriage_allowance: 0 + spouse: + age: 40 + adjusted_net_income: 0 + personal_allowance: 12_570 + marriage_allowance: 0 + benunits: + benunit: + members: [earner, spouse] + is_married: true + households: + household: + members: [earner, spouse] + region: NORTH_WEST + output: + net_income: [50_000, 0] + earned_taxable_income: [37_430, 0] + income_tax: [7_486, 0] + +- name: Marriage-neutral IT — a supplied adjusted net income gets the grossed-up gift back + period: 2026 + absolute_error_margin: 1 + reforms: + - policyengine_uk.reforms.cps.marriage_tax_reforms.marriage_neutral_it_reform + input: + people: + earner: + age: 40 + adjusted_net_income: 49_000 + gift_aid: 800 + personal_allowance: 12_570 + marriage_allowance: 0 + spouse: + age: 40 + adjusted_net_income: 0 + personal_allowance: 12_570 + marriage_allowance: 0 + benunits: + benunit: + members: [earner, spouse] + is_married: true + households: + household: + members: [earner, spouse] + region: NORTH_WEST + output: + # ANI already has the £1,000 grossed-up gift deducted (s.58), so net + # income is £50,000; the raised basic rate limit gives no relief here. + net_income: [50_000, 0] + adjusted_net_income: [49_000, 0] + earned_taxable_income: [37_430, 0] + income_tax: [7_486, 0] diff --git a/policyengine_uk/tests/test_capital_gains_tax_components.py b/policyengine_uk/tests/test_capital_gains_tax_components.py index 551d358315..23d5687c94 100644 --- a/policyengine_uk/tests/test_capital_gains_tax_components.py +++ b/policyengine_uk/tests/test_capital_gains_tax_components.py @@ -23,8 +23,11 @@ def legacy_capital_gains_tax(sim, year): """The single-schedule formula as it stood before #1858, on the same arrays. - Copied operation for operation so that any difference in the result is a - difference in the new formula, not in the comparison. + Its schedule allocation is copied operation for operation, so a difference + there is a difference in the new formula. Its income and allowance inputs + follow the current model (#2253 moved Gift Aid out of the allowances), so + it checks how gains are allocated to the schedules, not equivalence with + any earlier treatment of Gift Aid. """ hmrc = sim.tax_benefit_system.parameters(str(year)).gov.hmrc cgt = hmrc.cgt diff --git a/policyengine_uk/tests/test_gift_aid.py b/policyengine_uk/tests/test_gift_aid.py index 5adfb43b60..248b3a5b17 100644 --- a/policyengine_uk/tests/test_gift_aid.py +++ b/policyengine_uk/tests/test_gift_aid.py @@ -407,3 +407,72 @@ def test_explicit_adjusted_net_income_preserves_tax_base(ani, net_gift): assert supplied.calculate("net_income", YEAR)[0] == 50_000 assert supplied.calculate("earned_taxable_income", YEAR)[0] == 37_430 assert supplied.calculate("income_tax", YEAR)[0] == 7_486 + + +# The helper on its own, against each scale shape a caller passes it. Raising +# a threshold must equal recomputing the scale with that threshold raised; +# thresholds outside the range must not move, which a zero rate step (the +# model's extra UK bracket repeats 45%) would not reveal. +HELPER_SCALES = { + "UK, with an extra bracket at a distinct rate": ( + [0, 37_700, 125_140, 10_000_000], + [0.20, 0.40, 0.45, 0.50], + None, + ), + "Scotland, three rates (before 2018-19)": ( + [0, 31_500, 150_000], + [0.20, 0.40, 0.45], + "scottish", + ), + "Scotland, six rates": ( + [0, 3_967, 16_956, 31_092, 62_430, 112_570], + [0.19, 0.20, 0.21, 0.42, 0.45, 0.48], + "scottish", + ), +} + + +def _scale(thresholds, rates): + from policyengine_core.taxscales import MarginalRateTaxScale + + scale = MarginalRateTaxScale() + for threshold, rate in zip(thresholds, rates): + scale.add_bracket(threshold, rate) + return scale + + +@pytest.mark.parametrize("name", list(HELPER_SCALES)) +@settings(max_examples=200, deadline=None, derandomize=True) +@given( + income=st.floats(0, 12_000_000, allow_nan=False, allow_infinity=False), + gross_gift=st.floats(0, 2_000_000, allow_nan=False, allow_infinity=False), +) +def test_helper_equals_the_scale_with_its_limits_raised(name, income, gross_gift): + from policyengine_uk.utils.income_tax import ( + calc_income_tax_with_gift_aid, + first_extended_scottish_threshold, + ) + + thresholds, rates, kind = HELPER_SCALES[name] + scale = _scale(thresholds, rates) + if kind == "scottish": + first = first_extended_scottish_threshold(scale) + last = len(scale.thresholds) + # Three rates: raise the basic rate limit and above. Six: keep the + # starter limit and raise the basic rate's upper limit and above. + assert first == (1 if len(thresholds) == 3 else 2) + tax = calc_income_tax_with_gift_aid( + scale, np.array([income]), np.array([gross_gift]), first, last + ) + else: + # UK default: the basic and higher rate limits only. + first, last = 1, 3 + tax = calc_income_tax_with_gift_aid( + scale, np.array([income]), np.array([gross_gift]) + ) + raised = [ + threshold + gross_gift if first <= index < last else threshold + for index, threshold in enumerate(thresholds) + ] + expected = _scale(raised, rates).calc(np.array([income])) + assert tax[0] == pytest.approx(expected[0], rel=1e-9, abs=1e-6) diff --git a/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_household_income.py b/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_household_income.py index 816c2e2296..9c1e621f65 100644 --- a/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_household_income.py +++ b/policyengine_uk/variables/gov/dfe/maintenance_loans/maintenance_loan_household_income.py @@ -10,7 +10,7 @@ class maintenance_loan_household_income(Variable): "Student Finance England-style household income for maintenance loan assessment. " "This can be set explicitly in simulations. By default, the model uses a proxy: " "dependent students use sponsor income from a selected older household benefit unit, " - "plus their own benefit-unit income; otherwise the model uses benefit-unit adjusted net income. " + "plus their own benefit-unit income; otherwise the model uses benefit-unit net income before Gift Aid adjustments. " "A student is dependent only if none of the independence tests the model can see applies " "(Education (Student Support) Regulations 2011 Sch 4 para 2(1)): aged under 25 on the first " "day of the academic year (para 2(1)(a)), not in a couple (a proxy for para 2(1)(b)), and " From cffb3722a497b452da3e63336652a5f0fbfecaa0 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 12:27:08 -0400 Subject: [PATCH 03/27] Freeze the import-time heap and collect less often in the tests Each process that runs tests raises the first-generation GC threshold from 2,000 to 10,000 at start-up and calls gc.freeze() once the tests are collected, so collections stop walking the ~430,000 objects present after importing policyengine_uk. release_dead_variable_modules now also counts the frozen tax-benefit systems as live, since gc.get_objects() leaves frozen objects out; without that it would drop the import-time system's variable modules. Fixes #2316 Co-Authored-By: Claude Opus 5.5 --- changelog.d/2316.changed.md | 1 + policyengine_uk/tests/conftest.py | 49 +++++++++++++++++++ .../tests/test_variable_module_release.py | 34 +++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 changelog.d/2316.changed.md diff --git a/changelog.d/2316.changed.md b/changelog.d/2316.changed.md new file mode 100644 index 0000000000..ec4681ab4a --- /dev/null +++ b/changelog.d/2316.changed.md @@ -0,0 +1 @@ +- Cut the CPU the Python tests spend in garbage collection: each test process raises the first-generation GC threshold and freezes the objects that exist once the tests are collected (#2316). diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index ca783dd9ab..883c02fdad 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -4,6 +4,7 @@ import os import re import sys +import weakref import pytest from policyengine_core.taxbenefitsystems import TaxBenefitSystem @@ -102,6 +103,53 @@ def pytest_collection_modifyitems(config, items): item.add_marker(skip_microsimulation) +# Garbage collection. Most of the suite's time is in the property tests (the 51 +# *_properties.py files took 74% of pytest worker time on 10 October 2026), and +# each of their examples builds a Simulation, which loads about 1,100 variable +# modules again and clones the parameter tree. Python's cycle collector starts +# a collection every 2,000 net allocations of tracked objects (its default +# threshold), and its full collections walk every tracked object in the +# process, including the 430,000 or so present once policyengine_uk is +# imported. Each process that runs tests (every pytest-xdist worker) therefore: +# +# - starts a collection every 10,000 net allocations instead (GC_THRESHOLD, +# set at start-up; the older generations keep Python's defaults); +# - freezes every object that exists once the tests are collected +# (gc.freeze()): the modules, the import-time tax-benefit systems and the +# collected tests, all of which live for the whole session. The collector +# never scans or frees frozen objects. Objects created by tests are collected +# as before. +# +# On test_ni_class_1_age_properties.py (52 examples, no coverage) this cut CPU +# time from 88-96 seconds to 61, and raised peak memory from 0.79 GB to 0.89 GB +# (measured locally on 10 October 2026; freezing alone gave 77 seconds, the +# threshold alone 67). It changes when memory is reclaimed, not which tests run +# or what they compute. +GC_THRESHOLD = (10_000, 10, 10) + +# gc.get_objects() leaves out frozen objects, so release_dead_variable_modules +# looks up the systems that were frozen here. +_FROZEN_SYSTEMS = weakref.WeakSet() +_heap_frozen = False + + +def pytest_configure(config): + gc.set_threshold(*GC_THRESHOLD) + + +@pytest.hookimpl(trylast=True) +def pytest_collection_finish(session): + global _heap_frozen + if _heap_frozen: + return + gc.collect() + _FROZEN_SYSTEMS.update( + obj for obj in gc.get_objects() if isinstance(obj, TaxBenefitSystem) + ) + gc.freeze() + _heap_frozen = True + + # policyengine-core loads every variable file of each new tax-benefit system as # a module named "__" and registers it in # sys.modules, where it stays after the system is garbage collected. Every @@ -132,6 +180,7 @@ def release_dead_variable_modules(max_owners: int = 8) -> int: live = { str(id(obj)) for obj in gc.get_objects() if isinstance(obj, TaxBenefitSystem) } + live.update(str(id(system)) for system in _FROZEN_SYSTEMS) released = 0 for owner, names in owners.items(): if owner not in live: diff --git a/policyengine_uk/tests/test_variable_module_release.py b/policyengine_uk/tests/test_variable_module_release.py index 3d52036c73..9cf75de1fd 100644 --- a/policyengine_uk/tests/test_variable_module_release.py +++ b/policyengine_uk/tests/test_variable_module_release.py @@ -8,10 +8,16 @@ note to remove the workaround, rather than failing every PR after a core upgrade. If core keeps leaking but names the modules differently, it fails, because the release would then silently do nothing. + +The conftest also freezes the objects that exist once the tests are collected +(gc.freeze()). gc.get_objects() leaves frozen objects out, so the release must +still count a frozen system as live, and systems the tests build afterwards +must still be collected. """ import gc import sys +import weakref from pathlib import Path import pytest @@ -72,3 +78,31 @@ def test_dead_systems_modules_are_released_and_live_ones_kept(request): def test_release_waits_until_enough_systems_own_modules(request): release_dead_variable_modules = _conftest(request).release_dead_variable_modules assert release_dead_variable_modules(max_owners=10**9) == 0 + + +def test_frozen_systems_keep_their_modules(request): + conftest = _conftest(request) + # Built when policyengine_uk was imported, during collection, so frozen. + from policyengine_uk.tax_benefit_system import system + + assert gc.get_threshold() == conftest.GC_THRESHOLD + assert gc.get_freeze_count() > 0 + assert not any(obj is system for obj in gc.get_objects()) + module = type(system.variables["income_tax"]).__module__ + built_later = Simulation(situation=SITUATION).tax_benefit_system + if type(built_later.variables["income_tax"]).__module__ not in sys.modules: + pytest.skip("policyengine-core no longer keeps variable modules") + + conftest.release_dead_variable_modules(max_owners=0) + + kept = module in sys.modules + assert kept, f"{module}, of a live frozen system, was released" + + +def test_systems_built_after_the_freeze_are_still_collected(): + simulation = Simulation(situation=SITUATION) + system = weakref.ref(simulation.tax_benefit_system) + assert any(obj is system() for obj in gc.get_objects()) + del simulation + gc.collect() + assert system() is None From 057132c1ea6f9325416d00762493c905d37b48c6 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 14:35:33 -0400 Subject: [PATCH 04/27] Cite the CI measurement of the GC settings in conftest.py Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 883c02fdad..b9385a1ae8 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -120,11 +120,16 @@ def pytest_collection_modifyitems(config, items): # never scans or frees frozen objects. Objects created by tests are collected # as before. # -# On test_ni_class_1_age_properties.py (52 examples, no coverage) this cut CPU -# time from 88-96 seconds to 61, and raised peak memory from 0.79 GB to 0.89 GB -# (measured locally on 10 October 2026; freezing alone gave 77 seconds, the -# threshold alone 67). It changes when memory is reclaimed, not which tests run -# or what they compute. +# Measured on CI on 11 October 2026 (PR #2317: runs 38155711978 and +# 38158313028, each running the three shards under several settings at once +# from one commit): the workers' CPU time, summed over the shards, fell from +# 204 to 172 minutes, and their time in the collector from 83 to 55; the +# slowest shard took 20 minutes instead of 24. Peak runner memory did not +# change (15.3 GiB of 15.6). In the first run, freezing alone saved nothing, +# the threshold alone saved 13 minutes, and with a threshold of 25,000 two of +# the three shards stalled. Every test had the same outcome under each +# setting: this changes when memory is reclaimed, not which tests run or what +# they compute. GC_THRESHOLD = (10_000, 10, 10) # gc.get_objects() leaves out frozen objects, so release_dead_variable_modules From 50ebd887793dcd0fab4b99b6c622640d85ee7701 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 15:35:05 -0400 Subject: [PATCH 05/27] Keep systems alive past the freeze, guard it, and restore GC state at exit Review findings on #2317: - Two test modules built a Simulation at import (council tax uprating's region-to-country map, the childcare test's minimum care hours). Their systems died during collection, so their variable modules were frozen, then released and never freed (about 19,000 objects per worker). Read the hours from the package's parameters and build the map on first use. - conftest.py records any variable-module owner whose system died before the freeze, and test_no_system_dies_before_the_freeze fails on one. - Set only the first-generation threshold and keep the interpreter's other thresholds (CPython 3.14.4 defaults to (2000, 10, 0)). - Undo the freeze and threshold at unconfigure, for in-process runs. - Correct the comment: about 1.6 million objects are frozen per worker, and on the shard whose runner memory isn't full the largest worker's peak rose from 5.3 to 5.7 GiB. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 93 +++++++++++++------ .../tests/test_council_tax_uprating.py | 10 +- ..._uc_childcare_work_condition_properties.py | 11 +-- .../tests/test_variable_module_release.py | 18 +++- 4 files changed, 88 insertions(+), 44 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index b9385a1ae8..1ec459c814 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -107,39 +107,51 @@ def pytest_collection_modifyitems(config, items): # *_properties.py files took 74% of pytest worker time on 10 October 2026), and # each of their examples builds a Simulation, which loads about 1,100 variable # modules again and clones the parameter tree. Python's cycle collector starts -# a collection every 2,000 net allocations of tracked objects (its default -# threshold), and its full collections walk every tracked object in the -# process, including the 430,000 or so present once policyengine_uk is -# imported. Each process that runs tests (every pytest-xdist worker) therefore: +# a collection every 2,000 net allocations of tracked objects (the default in +# CPython 3.13, which CI uses), and its full collections walk every tracked +# object in the process, including the 430,000 or so present once +# policyengine_uk is imported. Each process that runs tests (every +# pytest-xdist worker) therefore: # -# - starts a collection every 10,000 net allocations instead (GC_THRESHOLD, -# set at start-up; the older generations keep Python's defaults); +# - starts a collection every 10,000 net allocations instead +# (GC_FIRST_GENERATION_THRESHOLD, set at start-up; the other thresholds keep +# the interpreter's defaults); # - freezes every object that exists once the tests are collected -# (gc.freeze()): the modules, the import-time tax-benefit systems and the -# collected tests, all of which live for the whole session. The collector -# never scans or frees frozen objects. Objects created by tests are collected -# as before. +# (gc.freeze(), about 1.6 million objects per worker): the modules, the +# tax-benefit systems built at import and the collected tests. The collector +# never scans frozen objects, so a frozen object that later becomes garbage +# is never freed. No tax-benefit system may therefore die before the freeze +# (test_variable_module_release.py checks this); build one in a fixture or on +# first use, not at import. Objects created by tests are collected as before. # -# Measured on CI on 11 October 2026 (PR #2317: runs 38155711978 and -# 38158313028, each running the three shards under several settings at once -# from one commit): the workers' CPU time, summed over the shards, fell from -# 204 to 172 minutes, and their time in the collector from 83 to 55; the -# slowest shard took 20 minutes instead of 24. Peak runner memory did not -# change (15.3 GiB of 15.6). In the first run, freezing alone saved nothing, -# the threshold alone saved 13 minutes, and with a threshold of 25,000 two of -# the three shards stalled. Every test had the same outcome under each -# setting: this changes when memory is reclaimed, not which tests run or what -# they compute. -GC_THRESHOLD = (10_000, 10, 10) +# Measured on CI on 11 October 2026 (runs and numbers in PR #2317): the +# workers' CPU time, summed over the three shards, fell from 204 to 172 +# minutes, and their time in the collector from 83 to 55; the slowest shard +# took 20 minutes instead of 24. The runner's memory was already full on two +# shards under every setting; on the third, the largest worker's peak rose from +# 5.3 to 5.7 GiB. Freezing alone saved nothing, the threshold alone saved 13 +# minutes of worker CPU, and with a threshold of 25,000 two of the three +# runners shut down. Every test had the same outcome under each setting: this +# changes when memory is reclaimed, not which tests run or what they compute. +GC_FIRST_GENERATION_THRESHOLD = 10_000 # gc.get_objects() leaves out frozen objects, so release_dead_variable_modules # looks up the systems that were frozen here. _FROZEN_SYSTEMS = weakref.WeakSet() +# Owners of variable modules whose system was built and had died by the +# freeze, and the owners already loaded when the session started (a process +# that runs pytest more than once keeps the previous session's). +_DEAD_AT_FREEZE = set() +_owners_at_start = set() _heap_frozen = False +_threshold_before = None def pytest_configure(config): - gc.set_threshold(*GC_THRESHOLD) + global _threshold_before + _owners_at_start.update(_variable_module_owners()) + _threshold_before = gc.get_threshold() + gc.set_threshold(GC_FIRST_GENERATION_THRESHOLD, *_threshold_before[1:]) @pytest.hookimpl(trylast=True) @@ -148,13 +160,30 @@ def pytest_collection_finish(session): if _heap_frozen: return gc.collect() - _FROZEN_SYSTEMS.update( - obj for obj in gc.get_objects() if isinstance(obj, TaxBenefitSystem) + systems = [obj for obj in gc.get_objects() if isinstance(obj, TaxBenefitSystem)] + _FROZEN_SYSTEMS.update(systems) + _DEAD_AT_FREEZE.update( + set(_variable_module_owners()) + - {str(id(system)) for system in systems} + - _owners_at_start ) gc.freeze() _heap_frozen = True +def pytest_unconfigure(config): + # Leave a process that runs pytest in-process (an IDE, a REPL) as it was. + global _heap_frozen + if _heap_frozen: + gc.unfreeze() + _FROZEN_SYSTEMS.clear() + _heap_frozen = False + _DEAD_AT_FREEZE.clear() + _owners_at_start.clear() + if _threshold_before is not None: + gc.set_threshold(*_threshold_before) + + # policyengine-core loads every variable file of each new tax-benefit system as # a module named "__" and registers it in # sys.modules, where it stays after the system is garbage collected. Every @@ -167,6 +196,16 @@ def pytest_collection_finish(session): _VARIABLE_MODULE_NAME = re.compile(r"(\d+)_-?\d+_") +def _variable_module_owners() -> dict: + """Map each owner id in loaded variable module names to those names.""" + owners = {} + for name in list(sys.modules): + match = _VARIABLE_MODULE_NAME.match(name) + if match: + owners.setdefault(match.group(1), []).append(name) + return owners + + def release_dead_variable_modules(max_owners: int = 8) -> int: """Remove dead tax-benefit systems' variable modules from sys.modules. @@ -174,11 +213,7 @@ def release_dead_variable_modules(max_owners: int = 8) -> int: modules, since finding the live systems scans every tracked object. Returns the number of modules removed. """ - owners = {} - for name in list(sys.modules): - match = _VARIABLE_MODULE_NAME.match(name) - if match: - owners.setdefault(match.group(1), []).append(name) + owners = _variable_module_owners() if len(owners) <= max_owners: return 0 gc.collect() diff --git a/policyengine_uk/tests/test_council_tax_uprating.py b/policyengine_uk/tests/test_council_tax_uprating.py index 318a124427..92ec4d0922 100644 --- a/policyengine_uk/tests/test_council_tax_uprating.py +++ b/policyengine_uk/tests/test_council_tax_uprating.py @@ -1,3 +1,4 @@ +import functools from types import SimpleNamespace import numpy as np @@ -20,10 +21,12 @@ COUNCIL_TAX_GROWTH = parameters.gov.economic_assumptions.yoy_growth.obr.council_tax +@functools.cache def _country_of_each_region(): # The model's own region-to-country mapping (the `country` variable), so # the expected growth below comes from a second path to the one the - # uprating code takes. + # uprating code takes. Built on first use, not at import: a system built + # and dropped while the tests are collected would be frozen by conftest.py. people = {f"person_{region}": {"age": 40} for region in REGIONS} households = { f"household_{region}": {"members": [f"person_{region}"], "region": region} @@ -34,14 +37,11 @@ def _country_of_each_region(): return dict(zip(REGIONS, countries)) -COUNTRY = _country_of_each_region() - - def _expected_growth(region, year): # Council tax exists in England, Scotland and Wales; OBR forecasts it for # each. Northern Ireland levies domestic rates instead, so its council tax # has no growth. An unknown region takes England's. - country = COUNTRY[region] + country = _country_of_each_region()[region] if country == "NORTHERN_IRELAND": return 0.0 series = "england" if country == "UNKNOWN" else country.lower() diff --git a/policyengine_uk/tests/test_uc_childcare_work_condition_properties.py b/policyengine_uk/tests/test_uc_childcare_work_condition_properties.py index 7b9547ff31..a1dfb4c39a 100644 --- a/policyengine_uk/tests/test_uc_childcare_work_condition_properties.py +++ b/policyengine_uk/tests/test_uc_childcare_work_condition_properties.py @@ -38,7 +38,7 @@ from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st -from policyengine_uk import Simulation +from policyengine_uk import Simulation, parameters YEAR = 2025 PROPERTY_SETTINGS = settings( @@ -144,12 +144,9 @@ def calculate(units, with_dependants=True): } -def min_care_hours(): - sim = Simulation(situation={"people": {"p": {"age": {YEAR: 30}}}}) - return sim.tax_benefit_system.parameters(YEAR).gov.dwp.carers_allowance.min_hours - - -MIN_CARE_HOURS = min_care_hours() +# Read from the package's parameters, not a Simulation's: a system built and +# dropped while the tests are collected would be frozen by conftest.py. +MIN_CARE_HOURS = parameters(YEAR).gov.dwp.carers_allowance.min_hours def meets_claimant_limb(member): diff --git a/policyengine_uk/tests/test_variable_module_release.py b/policyengine_uk/tests/test_variable_module_release.py index 9cf75de1fd..a0300c4538 100644 --- a/policyengine_uk/tests/test_variable_module_release.py +++ b/policyengine_uk/tests/test_variable_module_release.py @@ -11,8 +11,9 @@ The conftest also freezes the objects that exist once the tests are collected (gc.freeze()). gc.get_objects() leaves frozen objects out, so the release must -still count a frozen system as live, and systems the tests build afterwards -must still be collected. +still count a frozen system as live. A frozen object that later becomes +garbage is never freed, so no system may die before the freeze: its variable +modules would be frozen, then released and leaked. """ import gc @@ -85,7 +86,7 @@ def test_frozen_systems_keep_their_modules(request): # Built when policyengine_uk was imported, during collection, so frozen. from policyengine_uk.tax_benefit_system import system - assert gc.get_threshold() == conftest.GC_THRESHOLD + assert gc.get_threshold()[0] == conftest.GC_FIRST_GENERATION_THRESHOLD assert gc.get_freeze_count() > 0 assert not any(obj is system for obj in gc.get_objects()) module = type(system.variables["income_tax"]).__module__ @@ -106,3 +107,14 @@ def test_systems_built_after_the_freeze_are_still_collected(): del simulation gc.collect() assert system() is None + + +def test_no_system_dies_before_the_freeze(request): + dead = _conftest(request)._DEAD_AT_FREEZE + assert not dead, ( + f"{len(dead)} tax-benefit system(s) were built and dropped while the " + "tests were collected, so conftest.py froze their variable modules " + "and they will never be freed. A test module probably builds a " + "Simulation or CountryTaxBenefitSystem at import: build it in a " + "fixture or on first use instead." + ) From ce41e6552d3c3069fab7e8f0a34e3624ab003aa9 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 13:27:26 -0400 Subject: [PATCH 06/27] Compile each variable file once per test process Coverage's sys.monitoring core (COVERAGE_CORE=sysmon on CI) keeps every code object it has seen alive. policyengine-core compiles a system's variable files again for every new system, so each Simulation left about 2,500 code objects behind and the test shards grew to the runner's memory. The conftest now returns the code object compiled the first time for each variable file, so every system runs the same code with its own modules, classes and functions. Fixes #2330 Co-Authored-By: Claude Opus 5.5 --- changelog.d/2330.fixed.md | 1 + policyengine_uk/tests/conftest.py | 40 ++++++ .../tests/test_variable_code_sharing.py | 120 ++++++++++++++++++ 3 files changed, 161 insertions(+) create mode 100644 changelog.d/2330.fixed.md create mode 100644 policyengine_uk/tests/test_variable_code_sharing.py diff --git a/changelog.d/2330.fixed.md b/changelog.d/2330.fixed.md new file mode 100644 index 0000000000..54046df6d4 --- /dev/null +++ b/changelog.d/2330.fixed.md @@ -0,0 +1 @@ +- Stop the Python tests' memory growing with every Simulation under sys.monitoring coverage: each test process compiles each variable file once and every tax-benefit system runs that code (#2330). diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 1ec459c814..d4d018d778 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -5,6 +5,7 @@ import re import sys import weakref +from importlib.machinery import SourceFileLoader import pytest from policyengine_core.taxbenefitsystems import TaxBenefitSystem @@ -234,3 +235,42 @@ def release_dead_variable_modules(max_owners: int = 8) -> int: def _release_variable_modules_of_dead_systems(): yield release_dead_variable_modules() + + +# Coverage on CI is measured with sys.monitoring (COVERAGE_CORE=sysmon). That +# core keeps every code object it has seen alive until the process exits, so +# that it can key its records by id(code) (SysMonitor.code_objects in +# coverage/sysmon.py). policyengine-core compiles a system's variable files +# again for every new system, into new code objects, and every Simulation +# builds a new system, so each Simulation left about 2,500 code objects and +# 4 MB that were never freed. On CI the four workers of a shard grew to the +# whole runner's memory (15.3 of 15.6 GiB). +# +# A file's code object does not depend on the system that runs it, so each +# process compiles each variable file once and every later system runs that +# code object. Systems still get their own modules, classes and functions; +# only the immutable code is shared. Remove this once policyengine-core +# shares the code itself. +_variable_code = {} +_get_code = SourceFileLoader.get_code + + +def _get_shared_variable_code(loader, fullname): + """``SourceFileLoader.get_code``, compiling each variable file only once.""" + if not _VARIABLE_MODULE_NAME.match(fullname): + return _get_code(loader, fullname) + path = loader.get_filename(fullname) + stat = os.stat(path) + key = (path, stat.st_mtime_ns, stat.st_size) + code = _variable_code.get(key) + if code is None: + code = _variable_code[key] = _get_code(loader, fullname) + return code + + +def pytest_sessionstart(session): + SourceFileLoader.get_code = _get_shared_variable_code + + +def pytest_unconfigure(config): + SourceFileLoader.get_code = _get_code diff --git a/policyengine_uk/tests/test_variable_code_sharing.py b/policyengine_uk/tests/test_variable_code_sharing.py new file mode 100644 index 0000000000..f6364a3b63 --- /dev/null +++ b/policyengine_uk/tests/test_variable_code_sharing.py @@ -0,0 +1,120 @@ +"""The conftest compiles each variable file once and shares the code. + +policyengine-core compiles a system's variable files again for every new +system. Coverage's sys.monitoring core, which CI uses, keeps every code object +it has seen alive, so each Simulation used to leave its code objects behind. +The conftest makes every system run the code object compiled the first time. + +Systems must still get their own modules, classes and functions, run the code +of the file's current source, and compute the same results. If +policyengine-core starts sharing the code itself, the upstream check skips +with a note to remove the workaround. +""" + +import sys +from importlib.machinery import SourceFileLoader +from pathlib import Path + +import pytest + +from policyengine_uk import Simulation + +SITUATION = { + "people": {"adult": {"age": {2026: 40}, "employment_income": {2026: 30_000}}}, + "benunits": {"benunit": {"members": ["adult"]}}, + "households": {"household": {"members": ["adult"]}}, +} + + +def _conftest(request): + path = Path(__file__).with_name("conftest.py") + for plugin in request.config.pluginmanager.get_plugins(): + if getattr(plugin, "__file__", None) and Path(plugin.__file__) == path: + return plugin + raise AssertionError(f"{path} is not loaded as a plugin") + + +def _income_tax_formula(simulation): + variable = simulation.tax_benefit_system.variables["income_tax"] + return variable.get_formula() + + +def _coverage_code_objects(): + """Code objects coverage's sys.monitoring core holds, or None if inactive.""" + try: + import coverage + except ImportError: + return None + cov = coverage.Coverage.current() + collector = getattr(cov, "_collector", None) + if collector is None: + return None + lists = [getattr(tracer, "code_objects", None) for tracer in collector.tracers] + lists = [objects for objects in lists if objects is not None] + return sum(len(objects) for objects in lists) if lists else None + + +def test_systems_share_code_but_not_modules_classes_or_functions(): + first = Simulation(situation=SITUATION) + second = Simulation(situation=SITUATION) + first_class = type(first.tax_benefit_system.variables["income_tax"]) + second_class = type(second.tax_benefit_system.variables["income_tax"]) + first_formula = _income_tax_formula(first) + second_formula = _income_tax_formula(second) + + assert first_class is not second_class + assert ( + sys.modules[first_class.__module__] + is not (sys.modules[second_class.__module__]) + ) + assert first_formula is not second_formula + assert first_formula.__globals__ is not second_formula.__globals__ + assert first_formula.__code__ is second_formula.__code__ + assert first.calculate("income_tax", 2026) == second.calculate("income_tax", 2026) + + +def test_shared_code_is_compiled_from_the_current_source(request): + Simulation(situation=SITUATION) + shared = _conftest(request)._variable_code + variables_dir = Path(sys.modules["policyengine_uk"].__file__).parent / "variables" + # Core reads a directory's __init__.py for metadata and runs every other file. + files = { + str(path) for path in variables_dir.rglob("*.py") if path.name != "__init__.py" + } + + assert shared, "no variable code was shared" + for (path, mtime_ns, size), code in shared.items(): + stat = Path(path).stat() + assert (stat.st_mtime_ns, stat.st_size) == (mtime_ns, size), path + source = Path(path).read_text(encoding="utf-8") + assert code == compile(source, path, "exec", dont_inherit=True), path + assert {path for path, _, _ in shared} >= files + + +def test_policyengine_core_still_compiles_each_system_again(request, monkeypatch): + monkeypatch.setattr(SourceFileLoader, "get_code", _conftest(request)._get_code) + first = _income_tax_formula(Simulation(situation=SITUATION)) + second = _income_tax_formula(Simulation(situation=SITUATION)) + if first.__code__ is second.__code__: + pytest.skip( + "policyengine-core now shares variable code between systems; " + "remove _get_shared_variable_code from conftest.py and this file" + ) + assert first.__code__ == second.__code__ + + +def test_a_simulation_leaves_coverage_no_new_code_objects(request, monkeypatch): + if _coverage_code_objects() is None: + pytest.skip("coverage is not measuring with sys.monitoring") + Simulation(situation=SITUATION).calculate("income_tax", 2026) + + before = _coverage_code_objects() + Simulation(situation=SITUATION).calculate("income_tax", 2026) + shared = _coverage_code_objects() - before + monkeypatch.setattr(SourceFileLoader, "get_code", _conftest(request)._get_code) + Simulation(situation=SITUATION).calculate("income_tax", 2026) + unshared = _coverage_code_objects() - before - shared + + # Without sharing, a Simulation leaves about 2,500 code objects. + assert unshared > 1_000, unshared + assert shared < 100, shared From e41150e20bf6c3eb6c04ed10d1b844f4e1bded04 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 14:37:40 -0400 Subject: [PATCH 07/27] Release dead systems' variable modules as each new system loads A property test on CI had 165 systems' variable modules loaded at once, because dead systems' modules were released only after each test, and Hypothesis builds hundreds of systems while it shrinks a failing example. The loader hook now also runs release_dead_variable_modules() when a new system starts loading its variables, so at most max_owners dead systems' modules are held. Also record the CI before/after numbers in the comment. Co-Authored-By: Claude Opus 5.5 --- changelog.d/2330.fixed.md | 1 + policyengine_uk/tests/conftest.py | 31 ++++++++++++++----- .../tests/test_variable_module_release.py | 12 +++++++ 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/changelog.d/2330.fixed.md b/changelog.d/2330.fixed.md index 54046df6d4..8cffd45d4e 100644 --- a/changelog.d/2330.fixed.md +++ b/changelog.d/2330.fixed.md @@ -1 +1,2 @@ - Stop the Python tests' memory growing with every Simulation under sys.monitoring coverage: each test process compiles each variable file once and every tax-benefit system runs that code (#2330). +- Release dead tax-benefit systems' variable modules whenever a new system starts loading its variables in the Python tests, not only after each test. diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index d4d018d778..c84345ad58 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -242,23 +242,38 @@ def _release_variable_modules_of_dead_systems(): # that it can key its records by id(code) (SysMonitor.code_objects in # coverage/sysmon.py). policyengine-core compiles a system's variable files # again for every new system, into new code objects, and every Simulation -# builds a new system, so each Simulation left about 2,500 code objects and -# 4 MB that were never freed. On CI the four workers of a shard grew to the -# whole runner's memory (15.3 of 15.6 GiB). +# builds a new system, so each Simulation left about 2,500 code objects that +# were never freed. By the end of a CI shard each pytest-xdist worker's +# coverage held 0.8 to 1.5 million of them, and the shards peaked at 14.4 to +# 15.3 GiB of the runner's 15.6, using up to 1.8 GiB of swap (run 38159894687). # # A file's code object does not depend on the system that runs it, so each # process compiles each variable file once and every later system runs that # code object. Systems still get their own modules, classes and functions; -# only the immutable code is shared. Remove this once policyengine-core -# shares the code itself. +# only the immutable code is shared. In the same run this lowered the shards' +# peaks to 10.2 to 12.2 GiB with no swap, cut their pytest time by 15 to 21%, +# and left every test's outcome unchanged. Remove this once policyengine-core +# shares the code itself (PolicyEngine/policyengine-core#608). +# +# The same hook also releases dead systems' variable modules whenever a new +# system starts loading its variables, not only after each test: in that run +# one property test had 165 systems' modules loaded at once, and Hypothesis +# builds hundreds of systems while it shrinks a failing example. _variable_code = {} _get_code = SourceFileLoader.get_code +_loading_owner = None -def _get_shared_variable_code(loader, fullname): +def _get_variable_code(loader, fullname): """``SourceFileLoader.get_code``, compiling each variable file only once.""" - if not _VARIABLE_MODULE_NAME.match(fullname): + global _loading_owner + match = _VARIABLE_MODULE_NAME.match(fullname) + if not match: return _get_code(loader, fullname) + if match.group(1) != _loading_owner: + # A system loads all its variable files in turn, so this is a new one. + _loading_owner = match.group(1) + release_dead_variable_modules() path = loader.get_filename(fullname) stat = os.stat(path) key = (path, stat.st_mtime_ns, stat.st_size) @@ -269,7 +284,7 @@ def _get_shared_variable_code(loader, fullname): def pytest_sessionstart(session): - SourceFileLoader.get_code = _get_shared_variable_code + SourceFileLoader.get_code = _get_variable_code def pytest_unconfigure(config): diff --git a/policyengine_uk/tests/test_variable_module_release.py b/policyengine_uk/tests/test_variable_module_release.py index a0300c4538..57f6d4ec7e 100644 --- a/policyengine_uk/tests/test_variable_module_release.py +++ b/policyengine_uk/tests/test_variable_module_release.py @@ -118,3 +118,15 @@ def test_no_system_dies_before_the_freeze(request): "Simulation or CountryTaxBenefitSystem at import: build it in a " "fixture or on first use instead." ) + + +def test_dead_systems_modules_are_released_while_a_test_runs(request): + variable_module_owners = _conftest(request)._variable_module_owners + max_owners = 8 + owners = set() + for _ in range(3 * max_owners): + Simulation(situation=SITUATION).calculate("income_tax", 2026) + owners.add(len(variable_module_owners())) + # The release runs as each new system starts loading, and keeps at most + # max_owners systems' modules plus the new system's. + assert max(owners) <= max_owners + 1, sorted(owners) From a1774f0a27dfe8dbd87f775f9b6fe31b0530b4df Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 14:49:06 -0400 Subject: [PATCH 08/27] Return freed memory after each test file and drop the latest dataset On CI the worker that ran the microsimulation tests kept about 4 GiB of RSS after their simulations were gone, though locally only the 0.74 GiB cached dataset stayed referenced. The conftest now calls glibc's malloc_trim(0) once a file's fixtures are torn down, and the latest-data smoke test removes its unpinned dataset from the simulation cache when its module ends, since no other test uses it. Co-Authored-By: Claude Opus 5.5 --- changelog.d/2330.fixed.md | 1 + policyengine_uk/tests/conftest.py | 21 +++++++++ .../tests/test_latest_data_smoke.py | 7 ++- policyengine_uk/tests/test_malloc_trim.py | 43 +++++++++++++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 policyengine_uk/tests/test_malloc_trim.py diff --git a/changelog.d/2330.fixed.md b/changelog.d/2330.fixed.md index 8cffd45d4e..5706ee5f62 100644 --- a/changelog.d/2330.fixed.md +++ b/changelog.d/2330.fixed.md @@ -1,2 +1,3 @@ - Stop the Python tests' memory growing with every Simulation under sys.monitoring coverage: each test process compiles each variable file once and every tax-benefit system runs that code (#2330). - Release dead tax-benefit systems' variable modules whenever a new system starts loading its variables in the Python tests, not only after each test. +- Hand freed memory back to the system after each test file (glibc malloc_trim), and stop keeping the unpinned latest dataset cached after its smoke test. diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index c84345ad58..e13e5038b8 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -1,4 +1,5 @@ import argparse +import ctypes import gc import hashlib import os @@ -289,3 +290,23 @@ def pytest_sessionstart(session): def pytest_unconfigure(config): SourceFileLoader.get_code = _get_code + + +# Freed memory stays in the worker: on CI the worker that ran the +# microsimulation tests kept about 4 GiB of RSS after their simulations were +# gone (run 38159894687), though locally only the 0.74 GiB cached dataset was +# still referenced once they were deleted (tracemalloc). After each test +# file's fixtures are torn down, glibc's malloc_trim(0) hands free heap memory +# back to the system. Elsewhere this does nothing. +try: + _malloc_trim = ctypes.CDLL("libc.so.6").malloc_trim +except (OSError, AttributeError): + _malloc_trim = None + + +@pytest.hookimpl(trylast=True) +def pytest_runtest_teardown(item, nextitem): + # trylast: after pytest's own teardown, which finalizes the file's fixtures + # when the next test is in another file. + if _malloc_trim is not None and (nextitem is None or nextitem.path != item.path): + _malloc_trim(0) diff --git a/policyengine_uk/tests/test_latest_data_smoke.py b/policyengine_uk/tests/test_latest_data_smoke.py index aa9639cab9..4e32f537ec 100644 --- a/policyengine_uk/tests/test_latest_data_smoke.py +++ b/policyengine_uk/tests/test_latest_data_smoke.py @@ -20,6 +20,7 @@ import pytest from policyengine_uk import Microsimulation +from policyengine_uk.simulation import _url_dataset_cache LATEST_DATASET_URL = ( @@ -39,7 +40,11 @@ def sim() -> Microsimulation: """ with pytest.MonkeyPatch.context() as monkeypatch: monkeypatch.setenv("POLICYENGINE_UK_DEFAULT_DATASET", LATEST_DATASET_URL) - return Microsimulation() + simulation = Microsimulation() + yield simulation + # Only this file uses the unpinned dataset, so don't keep it cached (about + # 0.7 GiB) beside the pinned one for the rest of the test process. + _url_dataset_cache.pop(LATEST_DATASET_URL, None) def _weighted(sim: Microsimulation, variable: str, period: int = YEAR) -> float: diff --git a/policyengine_uk/tests/test_malloc_trim.py b/policyengine_uk/tests/test_malloc_trim.py new file mode 100644 index 0000000000..bcd9f88003 --- /dev/null +++ b/policyengine_uk/tests/test_malloc_trim.py @@ -0,0 +1,43 @@ +"""The conftest hands freed memory back to the system after each test file. + +It calls glibc's malloc_trim(0) once a file's last test has been torn down, +and only then. On Linux it must find the function, or the release would +silently do nothing. +""" + +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + + +def _conftest(request): + path = Path(__file__).with_name("conftest.py") + for plugin in request.config.pluginmanager.get_plugins(): + if getattr(plugin, "__file__", None) and Path(plugin.__file__) == path: + return plugin + raise AssertionError(f"{path} is not loaded as a plugin") + + +def test_malloc_trim_is_found_on_linux(request): + if not sys.platform.startswith("linux"): + pytest.skip("malloc_trim is part of glibc") + assert _conftest(request)._malloc_trim is not None + + +@pytest.mark.parametrize( + "next_file, trims", + [("test_a.py", False), ("test_b.py", True), (None, True)], + ids=["same file", "another file", "last test"], +) +def test_trims_only_after_a_files_last_test(request, monkeypatch, next_file, trims): + conftest = _conftest(request) + calls = [] + monkeypatch.setattr(conftest, "_malloc_trim", calls.append) + item = SimpleNamespace(path=Path("test_a.py")) + nextitem = None if next_file is None else SimpleNamespace(path=Path(next_file)) + + conftest.pytest_runtest_teardown(item, nextitem) + + assert calls == ([0] if trims else []) From 5051db8d17968d21ff7f54831cff3c517496e0c2 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 15:36:59 -0400 Subject: [PATCH 09/27] Install the variable code hook in pytest_configure pytest_sessionstart never reaches a conftest that pytest finds only while collecting, as with a bare `pytest` from the repository root, so the hook was not installed there and the sharing and release tests failed. pytest_configure is also called for conftests registered later. The loader is restored in the existing pytest_unconfigure (a second definition would replace it). Also read coverage's tracers with getattr in the test helper. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 12 ++++-------- policyengine_uk/tests/test_variable_code_sharing.py | 3 ++- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index e13e5038b8..e6b0a8cf28 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -154,6 +154,9 @@ def pytest_configure(config): _owners_at_start.update(_variable_module_owners()) _threshold_before = gc.get_threshold() gc.set_threshold(GC_FIRST_GENERATION_THRESHOLD, *_threshold_before[1:]) + # Below. pytest_configure, unlike pytest_sessionstart, also reaches this + # conftest when pytest finds it only while collecting (bare `pytest`). + SourceFileLoader.get_code = _get_variable_code @pytest.hookimpl(trylast=True) @@ -184,6 +187,7 @@ def pytest_unconfigure(config): _owners_at_start.clear() if _threshold_before is not None: gc.set_threshold(*_threshold_before) + SourceFileLoader.get_code = _get_code # policyengine-core loads every variable file of each new tax-benefit system as @@ -284,14 +288,6 @@ def _get_variable_code(loader, fullname): return code -def pytest_sessionstart(session): - SourceFileLoader.get_code = _get_variable_code - - -def pytest_unconfigure(config): - SourceFileLoader.get_code = _get_code - - # Freed memory stays in the worker: on CI the worker that ran the # microsimulation tests kept about 4 GiB of RSS after their simulations were # gone (run 38159894687), though locally only the 0.74 GiB cached dataset was diff --git a/policyengine_uk/tests/test_variable_code_sharing.py b/policyengine_uk/tests/test_variable_code_sharing.py index f6364a3b63..dddb869be2 100644 --- a/policyengine_uk/tests/test_variable_code_sharing.py +++ b/policyengine_uk/tests/test_variable_code_sharing.py @@ -49,7 +49,8 @@ def _coverage_code_objects(): collector = getattr(cov, "_collector", None) if collector is None: return None - lists = [getattr(tracer, "code_objects", None) for tracer in collector.tracers] + tracers = getattr(collector, "tracers", ()) + lists = [getattr(tracer, "code_objects", None) for tracer in tracers] lists = [objects for objects in lists if objects is not None] return sum(len(objects) for objects in lists) if lists else None From 7e96e72c4c5fe1caeca0b966ae7f241c04ce238f Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 15:17:52 -0400 Subject: [PATCH 10/27] Cite the CI measurement of the module release and malloc_trim Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index e6b0a8cf28..7a6930f55a 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -263,7 +263,8 @@ def _release_variable_modules_of_dead_systems(): # The same hook also releases dead systems' variable modules whenever a new # system starts loading its variables, not only after each test: in that run # one property test had 165 systems' modules loaded at once, and Hypothesis -# builds hundreds of systems while it shrinks a failing example. +# builds hundreds of systems while it shrinks a failing example. With this the +# shards peaked at 10.0 to 10.4 GiB (run 38165445604). _variable_code = {} _get_code = SourceFileLoader.get_code _loading_owner = None @@ -291,9 +292,12 @@ def _get_variable_code(loader, fullname): # Freed memory stays in the worker: on CI the worker that ran the # microsimulation tests kept about 4 GiB of RSS after their simulations were # gone (run 38159894687), though locally only the 0.74 GiB cached dataset was -# still referenced once they were deleted (tracemalloc). After each test -# file's fixtures are torn down, glibc's malloc_trim(0) hands free heap memory -# back to the system. Elsewhere this does nothing. +# still referenced once they were deleted (tracemalloc). So after each test +# file's fixtures are torn down, ask glibc to return free heap memory to the +# system (malloc_trim(0)); elsewhere this does nothing. With this, and +# test_latest_data_smoke.py no longer keeping its dataset cached, the shards +# peaked at 8.0 to 8.2 GiB instead of 10.0 to 10.4, in no more time, and two +# shards would peak at 8.1 (run 38165445604). try: _malloc_trim = ctypes.CDLL("libc.so.6").malloc_trim except (OSError, AttributeError): From 4a88e23a84e6b22b0515bd10d6a0ef0d3ac45ccc Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 16:32:33 -0400 Subject: [PATCH 11/27] Trigger the module release from add_variables_from_file and amortize it Opus review of 7e96e72c4: - The release ran from the SourceFileLoader hook, which policyengine-core#608 bypasses, so it would have stopped without notice. It now runs from TaxBenefitSystem.add_variables_from_file as each new system starts adding its variables. - With more than eight systems alive (an lru_cache of systems in a test module), every new system and every test scanned the heap and released nothing; a worker's later tests took 432 s instead of 219 s. The release now scans only once eight more systems own modules than were alive at the last scan, after tests as well as builds. - The release test assumed at most eight live systems and failed in a serial run; it now measures the live systems first. - Sharing tests skip, with a note, once policyengine-core shares the code; the note names the right function. - The malloc_trim test checks for glibc rather than Linux, malloc_trim gets its C signature, the loader restore leaves no copy on the subclass, the latest-data fixture keeps the cache when the latest URL is the pinned one, and comments say "about the same time" and "can build many more". Co-Authored-By: Claude Opus 5.5 --- changelog.d/2330.fixed.md | 2 +- policyengine_uk/tests/conftest.py | 94 ++++++++++++++----- .../tests/test_latest_data_smoke.py | 13 ++- policyengine_uk/tests/test_malloc_trim.py | 8 +- .../tests/test_variable_code_sharing.py | 47 +++++++--- .../tests/test_variable_module_release.py | 23 +++-- 6 files changed, 130 insertions(+), 57 deletions(-) diff --git a/changelog.d/2330.fixed.md b/changelog.d/2330.fixed.md index 5706ee5f62..a84eac6a62 100644 --- a/changelog.d/2330.fixed.md +++ b/changelog.d/2330.fixed.md @@ -1,3 +1,3 @@ - Stop the Python tests' memory growing with every Simulation under sys.monitoring coverage: each test process compiles each variable file once and every tax-benefit system runs that code (#2330). -- Release dead tax-benefit systems' variable modules whenever a new system starts loading its variables in the Python tests, not only after each test. +- Release dead tax-benefit systems' variable modules in the Python tests once eight more systems own modules than were alive at the last release, checked as each new system starts adding its variables as well as after each test. - Hand freed memory back to the system after each test file (glibc malloc_trim), and stop keeping the unpinned latest dataset cached after its smoke test. diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 7a6930f55a..2c4d6f20d6 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -154,9 +154,10 @@ def pytest_configure(config): _owners_at_start.update(_variable_module_owners()) _threshold_before = gc.get_threshold() gc.set_threshold(GC_FIRST_GENERATION_THRESHOLD, *_threshold_before[1:]) - # Below. pytest_configure, unlike pytest_sessionstart, also reaches this - # conftest when pytest finds it only while collecting (bare `pytest`). - SourceFileLoader.get_code = _get_variable_code + # The variable code and module hooks defined below. pytest_configure, unlike + # pytest_sessionstart, also reaches a conftest that pytest finds only while + # collecting (a bare `pytest` from the repository root). + _install_variable_hooks() @pytest.hookimpl(trylast=True) @@ -187,7 +188,7 @@ def pytest_unconfigure(config): _owners_at_start.clear() if _threshold_before is not None: gc.set_threshold(*_threshold_before) - SourceFileLoader.get_code = _get_code + _remove_variable_hooks() # policyengine-core loads every variable file of each new tax-benefit system as @@ -236,10 +237,28 @@ def release_dead_variable_modules(max_owners: int = 8) -> int: return released +# A test or a new system triggers the release only once ``max_new`` systems +# have been built since the last release found the live ones, so a process +# that keeps more than eight systems alive (an lru_cache of systems in a test +# module) doesn't scan every tracked object after every test and every build. +_live_owners = 0 + + +def release_after_new_systems(max_new: int = 8) -> int: + """Release dead systems' variable modules once ``max_new`` more systems own + modules than were alive at the last release. Returns the number removed.""" + global _live_owners + if len(_variable_module_owners()) <= _live_owners + max_new: + return 0 + released = release_dead_variable_modules(max_owners=0) + _live_owners = len(_variable_module_owners()) + return released + + @pytest.fixture(autouse=True) def _release_variable_modules_of_dead_systems(): yield - release_dead_variable_modules() + release_after_new_systems() # Coverage on CI is measured with sys.monitoring (COVERAGE_CORE=sysmon). That @@ -257,29 +276,17 @@ def _release_variable_modules_of_dead_systems(): # code object. Systems still get their own modules, classes and functions; # only the immutable code is shared. In the same run this lowered the shards' # peaks to 10.2 to 12.2 GiB with no swap, cut their pytest time by 15 to 21%, -# and left every test's outcome unchanged. Remove this once policyengine-core -# shares the code itself (PolicyEngine/policyengine-core#608). -# -# The same hook also releases dead systems' variable modules whenever a new -# system starts loading its variables, not only after each test: in that run -# one property test had 165 systems' modules loaded at once, and Hypothesis -# builds hundreds of systems while it shrinks a failing example. With this the -# shards peaked at 10.0 to 10.4 GiB (run 38165445604). +# and left every test's outcome unchanged. Remove _get_variable_code once +# policyengine-core shares the code itself (PolicyEngine/policyengine-core#608). _variable_code = {} _get_code = SourceFileLoader.get_code -_loading_owner = None +_get_code_was_own = "get_code" in SourceFileLoader.__dict__ def _get_variable_code(loader, fullname): """``SourceFileLoader.get_code``, compiling each variable file only once.""" - global _loading_owner - match = _VARIABLE_MODULE_NAME.match(fullname) - if not match: + if not _VARIABLE_MODULE_NAME.match(fullname): return _get_code(loader, fullname) - if match.group(1) != _loading_owner: - # A system loads all its variable files in turn, so this is a new one. - _loading_owner = match.group(1) - release_dead_variable_modules() path = loader.get_filename(fullname) stat = os.stat(path) key = (path, stat.st_mtime_ns, stat.st_size) @@ -289,17 +296,56 @@ def _get_variable_code(loader, fullname): return code +# Dead systems' variable modules are also released as each new system starts +# adding its variables, not only after each test: in that run one property +# test had 165 systems' modules loaded at once, and Hypothesis can build many +# more while it shrinks a failing example. With this the shards peaked at 10.0 +# to 10.4 GiB (run 38165445604). Keep this until policyengine-core stops +# leaking the modules (PolicyEngine/policyengine-core#520). +_add_variables_from_file = TaxBenefitSystem.add_variables_from_file +_adding_system = None + + +def _add_variables_from_file_releasing(system, file_path): + """``TaxBenefitSystem.add_variables_from_file``, releasing dead systems' + modules when a new system starts adding its variables.""" + global _adding_system + if id(system) != _adding_system: + # A system adds all its variable files in turn, so this is a new one. + _adding_system = id(system) + release_after_new_systems() + return _add_variables_from_file(system, file_path) + + +def _install_variable_hooks(): + SourceFileLoader.get_code = _get_variable_code + TaxBenefitSystem.add_variables_from_file = _add_variables_from_file_releasing + + +def _remove_variable_hooks(): + if _get_code_was_own: + SourceFileLoader.get_code = _get_code + elif "get_code" in SourceFileLoader.__dict__: + # Inherited from SourceLoader: leave no copy on the subclass. + del SourceFileLoader.get_code + TaxBenefitSystem.add_variables_from_file = _add_variables_from_file + + # Freed memory stays in the worker: on CI the worker that ran the # microsimulation tests kept about 4 GiB of RSS after their simulations were # gone (run 38159894687), though locally only the 0.74 GiB cached dataset was # still referenced once they were deleted (tracemalloc). So after each test # file's fixtures are torn down, ask glibc to return free heap memory to the -# system (malloc_trim(0)); elsewhere this does nothing. With this, and +# system (malloc_trim(0)); memory still awaiting the cycle collector goes back +# at a later file. Elsewhere this does nothing. With this, and # test_latest_data_smoke.py no longer keeping its dataset cached, the shards -# peaked at 8.0 to 8.2 GiB instead of 10.0 to 10.4, in no more time, and two -# shards would peak at 8.1 (run 38165445604). +# peaked at 8.0 to 8.2 GiB instead of 10.0 to 10.4, in about the same time +# (pytest wall 776 / 952 / 637 s against 788 / 902 / 581), and two shards +# would peak at 8.1 (run 38165445604). try: _malloc_trim = ctypes.CDLL("libc.so.6").malloc_trim + _malloc_trim.argtypes = [ctypes.c_size_t] + _malloc_trim.restype = ctypes.c_int except (OSError, AttributeError): _malloc_trim = None diff --git a/policyengine_uk/tests/test_latest_data_smoke.py b/policyengine_uk/tests/test_latest_data_smoke.py index 4e32f537ec..51076d0190 100644 --- a/policyengine_uk/tests/test_latest_data_smoke.py +++ b/policyengine_uk/tests/test_latest_data_smoke.py @@ -16,6 +16,9 @@ from __future__ import annotations +import os +from collections.abc import Iterator + import numpy as np import pytest @@ -30,7 +33,7 @@ @pytest.fixture(scope="module") -def sim() -> Microsimulation: +def sim() -> Iterator[Microsimulation]: """Simulation built against the unpinned latest dataset. Overrides any pinned-version dataset set in conftest.py so the test @@ -42,9 +45,11 @@ def sim() -> Microsimulation: monkeypatch.setenv("POLICYENGINE_UK_DEFAULT_DATASET", LATEST_DATASET_URL) simulation = Microsimulation() yield simulation - # Only this file uses the unpinned dataset, so don't keep it cached (about - # 0.7 GiB) beside the pinned one for the rest of the test process. - _url_dataset_cache.pop(LATEST_DATASET_URL, None) + # Only this file uses the unpinned dataset, so don't keep it cached beside + # the pinned one for the rest of the test process (a cached copy of the + # pinned 2023-24 dataset held 0.74 GiB, measured locally). + if os.environ.get("POLICYENGINE_UK_DEFAULT_DATASET") != LATEST_DATASET_URL: + _url_dataset_cache.pop(LATEST_DATASET_URL, None) def _weighted(sim: Microsimulation, variable: str, period: int = YEAR) -> float: diff --git a/policyengine_uk/tests/test_malloc_trim.py b/policyengine_uk/tests/test_malloc_trim.py index bcd9f88003..df4d2b2ca1 100644 --- a/policyengine_uk/tests/test_malloc_trim.py +++ b/policyengine_uk/tests/test_malloc_trim.py @@ -1,11 +1,11 @@ """The conftest hands freed memory back to the system after each test file. It calls glibc's malloc_trim(0) once a file's last test has been torn down, -and only then. On Linux it must find the function, or the release would +and only then. With glibc it must find the function, or the release would silently do nothing. """ -import sys +import platform from pathlib import Path from types import SimpleNamespace @@ -20,8 +20,8 @@ def _conftest(request): raise AssertionError(f"{path} is not loaded as a plugin") -def test_malloc_trim_is_found_on_linux(request): - if not sys.platform.startswith("linux"): +def test_malloc_trim_is_found_with_glibc(request): + if platform.libc_ver()[0] != "glibc": pytest.skip("malloc_trim is part of glibc") assert _conftest(request)._malloc_trim is not None diff --git a/policyengine_uk/tests/test_variable_code_sharing.py b/policyengine_uk/tests/test_variable_code_sharing.py index dddb869be2..7445f4ab9b 100644 --- a/policyengine_uk/tests/test_variable_code_sharing.py +++ b/policyengine_uk/tests/test_variable_code_sharing.py @@ -6,9 +6,9 @@ The conftest makes every system run the code object compiled the first time. Systems must still get their own modules, classes and functions, run the code -of the file's current source, and compute the same results. If -policyengine-core starts sharing the code itself, the upstream check skips -with a note to remove the workaround. +of the file's current source, and compute the same results. Once +policyengine-core shares the code itself (PolicyEngine/policyengine-core#608), +the tests of the workaround skip with a note to remove it. """ import sys @@ -34,6 +34,27 @@ def _conftest(request): raise AssertionError(f"{path} is not loaded as a plugin") +@pytest.fixture(scope="module") +def core_shares_code(request): + """Whether policyengine-core itself gives two systems the same code.""" + from policyengine_uk.tax_benefit_system import CountryTaxBenefitSystem + + with pytest.MonkeyPatch.context() as monkeypatch: + monkeypatch.setattr(SourceFileLoader, "get_code", _conftest(request)._get_code) + first, second = CountryTaxBenefitSystem(), CountryTaxBenefitSystem() + first = first.variables["income_tax"].get_formula().__code__ + second = second.variables["income_tax"].get_formula().__code__ + return first is second + + +def _skip_if_core_shares(core_shares_code): + if core_shares_code: + pytest.skip( + "policyengine-core now shares variable code between systems; " + "remove _get_variable_code from conftest.py and the tests of it" + ) + + def _income_tax_formula(simulation): variable = simulation.tax_benefit_system.variables["income_tax"] return variable.get_formula() @@ -74,7 +95,8 @@ def test_systems_share_code_but_not_modules_classes_or_functions(): assert first.calculate("income_tax", 2026) == second.calculate("income_tax", 2026) -def test_shared_code_is_compiled_from_the_current_source(request): +def test_shared_code_is_compiled_from_the_current_source(request, core_shares_code): + _skip_if_core_shares(core_shares_code) Simulation(situation=SITUATION) shared = _conftest(request)._variable_code variables_dir = Path(sys.modules["policyengine_uk"].__file__).parent / "variables" @@ -92,19 +114,14 @@ def test_shared_code_is_compiled_from_the_current_source(request): assert {path for path, _, _ in shared} >= files -def test_policyengine_core_still_compiles_each_system_again(request, monkeypatch): - monkeypatch.setattr(SourceFileLoader, "get_code", _conftest(request)._get_code) - first = _income_tax_formula(Simulation(situation=SITUATION)) - second = _income_tax_formula(Simulation(situation=SITUATION)) - if first.__code__ is second.__code__: - pytest.skip( - "policyengine-core now shares variable code between systems; " - "remove _get_shared_variable_code from conftest.py and this file" - ) - assert first.__code__ == second.__code__ +def test_policyengine_core_still_compiles_each_system_again(core_shares_code): + _skip_if_core_shares(core_shares_code) -def test_a_simulation_leaves_coverage_no_new_code_objects(request, monkeypatch): +def test_a_simulation_leaves_coverage_no_new_code_objects( + request, monkeypatch, core_shares_code +): + _skip_if_core_shares(core_shares_code) if _coverage_code_objects() is None: pytest.skip("coverage is not measuring with sys.monitoring") Simulation(situation=SITUATION).calculate("income_tax", 2026) diff --git a/policyengine_uk/tests/test_variable_module_release.py b/policyengine_uk/tests/test_variable_module_release.py index 57f6d4ec7e..acf96d9952 100644 --- a/policyengine_uk/tests/test_variable_module_release.py +++ b/policyengine_uk/tests/test_variable_module_release.py @@ -120,13 +120,18 @@ def test_no_system_dies_before_the_freeze(request): ) -def test_dead_systems_modules_are_released_while_a_test_runs(request): - variable_module_owners = _conftest(request)._variable_module_owners - max_owners = 8 - owners = set() - for _ in range(3 * max_owners): +def test_dead_systems_modules_are_released_while_a_test_runs(request, monkeypatch): + conftest = _conftest(request) + max_new = 8 + # Find the systems alive now; other test modules may keep many (caches). + monkeypatch.setattr(conftest, "_live_owners", 0) + conftest.release_after_new_systems(max_new=0) + live = len(conftest._variable_module_owners()) + owners = [] + for _ in range(max_new + 3): Simulation(situation=SITUATION).calculate("income_tax", 2026) - owners.add(len(variable_module_owners())) - # The release runs as each new system starts loading, and keeps at most - # max_owners systems' modules plus the new system's. - assert max(owners) <= max_owners + 1, sorted(owners) + owners.append(len(conftest._variable_module_owners())) + # Released as each new system starts adding its variables, so at most + # max_new dead systems' modules beyond the live ones, plus the newest + # system's. Without the release the last count would be live + 11. + assert max(owners) <= live + max_new + 1, (live, owners) From 591f90fd26b1fa7597881606b0c84edb8047532d Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 16:41:43 -0400 Subject: [PATCH 12/27] Test the release's bookkeeping and that it runs inside a test Opus delta review of 4a88e23a8 (approved) left three nits: the comment was off by one (a scan needs more than max_new owners beyond the live ones); the release test could pass if builds reused dead systems' ids, so it now also asserts a release ran; and nothing pinned the amortization, so a Hypothesis test models builds, deaths and releases with faked owner counts and checks that a scan happens exactly when more than max_new owners are beyond the live ones and that at most max_new are left after. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 9 +-- .../tests/test_variable_module_release.py | 63 +++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 2c4d6f20d6..91329219ea 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -237,16 +237,17 @@ def release_dead_variable_modules(max_owners: int = 8) -> int: return released -# A test or a new system triggers the release only once ``max_new`` systems -# have been built since the last release found the live ones, so a process +# A test or a new system triggers the release only once more than ``max_new`` +# systems own modules beyond those alive at the last release, so a process # that keeps more than eight systems alive (an lru_cache of systems in a test # module) doesn't scan every tracked object after every test and every build. _live_owners = 0 def release_after_new_systems(max_new: int = 8) -> int: - """Release dead systems' variable modules once ``max_new`` more systems own - modules than were alive at the last release. Returns the number removed.""" + """Release dead systems' variable modules once more than ``max_new`` systems + own modules beyond those alive at the last release. Returns the number + removed.""" global _live_owners if len(_variable_module_owners()) <= _live_owners + max_new: return 0 diff --git a/policyengine_uk/tests/test_variable_module_release.py b/policyengine_uk/tests/test_variable_module_release.py index acf96d9952..9af463d13d 100644 --- a/policyengine_uk/tests/test_variable_module_release.py +++ b/policyengine_uk/tests/test_variable_module_release.py @@ -22,6 +22,8 @@ from pathlib import Path import pytest +from hypothesis import given, settings +from hypothesis import strategies as st from policyengine_uk import Simulation @@ -127,6 +129,13 @@ def test_dead_systems_modules_are_released_while_a_test_runs(request, monkeypatc monkeypatch.setattr(conftest, "_live_owners", 0) conftest.release_after_new_systems(max_new=0) live = len(conftest._variable_module_owners()) + scans = [] + release = conftest.release_dead_variable_modules + monkeypatch.setattr( + conftest, + "release_dead_variable_modules", + lambda **kwargs: scans.append(kwargs) or release(**kwargs), + ) owners = [] for _ in range(max_new + 3): Simulation(situation=SITUATION).calculate("income_tax", 2026) @@ -134,4 +143,58 @@ def test_dead_systems_modules_are_released_while_a_test_runs(request, monkeypatc # Released as each new system starts adding its variables, so at most # max_new dead systems' modules beyond the live ones, plus the newest # system's. Without the release the last count would be live + 11. + assert scans, "no release ran while the systems were built" assert max(owners) <= live + max_new + 1, (live, owners) + + +# Model of the release's bookkeeping, without building systems: each step +# builds a system that stays alive or dies at once, kills live systems, or +# runs the release. After every release, at most max_new owners are beyond +# those alive at the last scan, and a scan happens exactly when more are. +_steps = st.lists( + st.one_of( + st.tuples(st.just("build"), st.booleans()), + st.tuples(st.just("die"), st.integers(0, 5)), + st.tuples(st.just("release"), st.just(0)), + ), + max_size=60, +) + + +@settings(max_examples=200, deadline=None) +@given(steps=_steps, max_new=st.integers(0, 10)) +def test_release_scans_only_past_max_new_new_owners(request, steps, max_new): + conftest = _conftest(request) + live, dead, scans = 1, 0, [] + + def owners(): + return {str(i): [] for i in range(live + dead)} + + def release_dead(max_owners): + nonlocal dead + released, dead = dead, 0 + scans.append(max_owners) + return released + + with pytest.MonkeyPatch.context() as monkeypatch: + monkeypatch.setattr(conftest, "_variable_module_owners", owners) + monkeypatch.setattr(conftest, "release_dead_variable_modules", release_dead) + monkeypatch.setattr(conftest, "_live_owners", 0) + for action, value in steps: + if action == "build": + live, dead = (live + 1, dead) if value else (live, dead + 1) + elif action == "die": + moved = min(value, live) + live, dead = live - moved, dead + moved + else: + last_live = conftest._live_owners + due = live + dead > last_live + max_new + before = len(scans) + released = conftest.release_after_new_systems(max_new=max_new) + assert (len(scans) > before) == due + if due: + assert scans[-1] == 0 and dead == 0 and released >= 0 + assert conftest._live_owners == live + else: + assert released == 0 + assert live + dead <= conftest._live_owners + max_new From 4ee6936edf187d2efb19ffc843970c6064cc474c Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 17:41:05 -0400 Subject: [PATCH 13/27] Cite the final CI measurement in conftest.py Run 38173323996, at 591f90fd2 with all of PR #2331's changes, peaked at 7.6 to 8.7 GiB; the release comment no longer attributes run 38165445604's figure to the amortized trigger, which that run did not use. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 91329219ea..62b57652a8 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -300,9 +300,11 @@ def _get_variable_code(loader, fullname): # Dead systems' variable modules are also released as each new system starts # adding its variables, not only after each test: in that run one property # test had 165 systems' modules loaded at once, and Hypothesis can build many -# more while it shrinks a failing example. With this the shards peaked at 10.0 -# to 10.4 GiB (run 38165445604). Keep this until policyengine-core stops -# leaking the modules (PolicyEngine/policyengine-core#520). +# more while it shrinks a failing example. In run 38165445604 an earlier form +# of this release, which scanned at every new system once more than eight +# systems owned modules, brought the shards' peaks from 10.2-12.2 GiB to +# 10.0-10.4. Keep this until policyengine-core stops leaking the modules +# (PolicyEngine/policyengine-core#520). _add_variables_from_file = TaxBenefitSystem.add_variables_from_file _adding_system = None @@ -342,7 +344,8 @@ def _remove_variable_hooks(): # test_latest_data_smoke.py no longer keeping its dataset cached, the shards # peaked at 8.0 to 8.2 GiB instead of 10.0 to 10.4, in about the same time # (pytest wall 776 / 952 / 637 s against 788 / 902 / 581), and two shards -# would peak at 8.1 (run 38165445604). +# would peak at 8.1 (run 38165445604). With everything in this file the three +# shards peaked at 7.6 to 8.7 GiB (run 38173323996). try: _malloc_trim = ctypes.CDLL("libc.so.6").malloc_trim _malloc_trim.argtypes = [ctypes.c_size_t] From 4d472d9c58bfd9b5ea9e1823a5a3d14092fa54a1 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 17:43:23 -0400 Subject: [PATCH 14/27] Cite each run for its own figure in the release comment Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 62b57652a8..dfab30455d 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -300,10 +300,11 @@ def _get_variable_code(loader, fullname): # Dead systems' variable modules are also released as each new system starts # adding its variables, not only after each test: in that run one property # test had 165 systems' modules loaded at once, and Hypothesis can build many -# more while it shrinks a failing example. In run 38165445604 an earlier form -# of this release, which scanned at every new system once more than eight -# systems owned modules, brought the shards' peaks from 10.2-12.2 GiB to -# 10.0-10.4. Keep this until policyengine-core stops leaking the modules +# more while it shrinks a failing example. With an earlier form of this +# release, which scanned at every new system once more than eight systems +# owned modules, the shards peaked at 10.0-10.4 GiB (run 38165445604), against +# 10.2-12.2 with code sharing alone (run 38159894687). Keep this until +# policyengine-core stops leaking the modules # (PolicyEngine/policyengine-core#520). _add_variables_from_file = TaxBenefitSystem.add_variables_from_file _adding_system = None From 70851abb1c6d13e0930497cd457886e5c4f8b3a3 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 21:01:31 -0400 Subject: [PATCH 15/27] Count statutory sick, maternity and paternity pay as UC earned income UC Regs 2013 reg. 55(4) treats statutory sick, maternity and paternity pay (and adoption, shared parental, parental bereavement and neonatal care pay, which the model has no variables for) as employed earnings, and reg. 55(5) deducts tax and primary Class 1 NI from "the amount of general earnings or benefits specified in paragraphs (2) to (4)". The model deducted the tax and NI on these payments from earned income but left the payments out, so Universal Credit rose by 15.4p per pound of sick or maternity pay above the thresholds. Adds YAML cases and Hypothesis properties: the award never rises with a claimant's statutory pay (two intended exceptions executed and labelled: reg. 32(2)(b) childcare work condition, ITA 2007 s. 55B(2)(b) Marriage Allowance cliff), sick and maternity pay count exactly as wages, earned income adds up, and a dependant's pay is not the claimants'. Fixes #2337 Co-Authored-By: Claude Opus 5.5 --- changelog.d/2337.fixed.md | 1 + .../income/uc_statutory_pay.yaml | 203 +++++++ .../tests/test_uc_statutory_pay_properties.py | 497 ++++++++++++++++++ .../uc_individual_earned_income_before_mif.py | 26 +- 4 files changed, 722 insertions(+), 5 deletions(-) create mode 100644 changelog.d/2337.fixed.md create mode 100644 policyengine_uk/tests/policy/baseline/finance/benefit/family/universal_credit/income/uc_statutory_pay.yaml create mode 100644 policyengine_uk/tests/test_uc_statutory_pay_properties.py diff --git a/changelog.d/2337.fixed.md b/changelog.d/2337.fixed.md new file mode 100644 index 0000000000..17d3f13dbf --- /dev/null +++ b/changelog.d/2337.fixed.md @@ -0,0 +1 @@ +- Counted statutory sick, maternity and paternity pay as Universal Credit earned income (UC Regs 2013 reg. 55(4)). The income tax and National Insurance on them were already deducted from earned income, so Universal Credit rose as the payments rose. diff --git a/policyengine_uk/tests/policy/baseline/finance/benefit/family/universal_credit/income/uc_statutory_pay.yaml b/policyengine_uk/tests/policy/baseline/finance/benefit/family/universal_credit/income/uc_statutory_pay.yaml new file mode 100644 index 0000000000..910c62df7d --- /dev/null +++ b/policyengine_uk/tests/policy/baseline/finance/benefit/family/universal_credit/income/uc_statutory_pay.yaml @@ -0,0 +1,203 @@ +# Universal Credit treats statutory sick, maternity and paternity pay as +# employed earnings (UC Regs 2013 reg. 55(4)(a) to (c)). Reg. 55(5) takes +# relievable pension contributions, income tax and primary Class 1 National +# Insurance off "the amount of general earnings or benefits specified in +# paragraphs (2) to (4)", so the tax on the payments comes off with the rest. +# Reg. 55(4)(e) to (h) list statutory adoption, shared parental, parental +# bereavement and neonatal care pay too; the model has no variables for them. +# +# Before this was fixed the payments were left out of earned income while the +# income tax and National Insurance on them were still deducted. Each pound of +# sick or maternity pay above the thresholds then raised the award by +# 0.55 x (20% + 8%) = 15.4p; each pound of paternity pay by 0.55 x 8% = 4.4p. +# +# Hand calculations use the 2026-27 rates: +# - personal allowance 12,570; basic rate 20%; +# - primary Class 1 NI 8% above the primary threshold of 241.73 a week +# (12,569.96 a year); +# - UC standard allowance 424.90 a month (single, 25 or over) or 666.97 +# (couple, one 25 or over); child element 303.94 a month; work allowance +# 427 a month with housing costs; taper 55%. + +- name: Statutory sick pay counts as earnings, and the tax on it comes off with the rest + period: 2026 + absolute_error_margin: 0.01 + input: + people: + parent: + age: 30 + employment_income: 20_000 + statutory_sick_pay: 2_000 + child: + age: 5 + benunits: + benunit: + members: [parent, child] + households: + household: + members: [parent, child] + tenure_type: RENT_FROM_COUNCIL + rent: 9_600 + output: + # (20,000 + 2,000 - 12,570) x 20%. + income_tax: [1_886, 0] + uc_income_tax_on_earnings: [1_886, 0] + # (22,000 - 12,569.96) x 8%. + uc_national_insurance_on_earnings: [754.40, 0] + # 22,000 - 1,886 - 754.40. Before this fix: 20,000 - 1,886 - 754.40 + # = 17,359.60. + uc_individual_earned_income: [19_359.60, 0] + # Less the work allowance of 427 x 12 = 5,124. + uc_earned_income: 14_235.60 + # 424.90 x 12 + 303.94 x 12 + 9,600 = 5,098.80 + 3,647.28 + 9,600. + uc_maximum_amount: 18_346.08 + # 18,346.08 - 0.55 x 14,235.60. Before this fix: 11,616.50, which is + # 308 more than the 11,308.50 the same parent gets with no sick pay. + universal_credit: 10_516.50 + +- name: Statutory maternity pay counts as earnings in the same way + period: 2026 + absolute_error_margin: 0.01 + input: + people: + parent: + age: 30 + employment_income: 20_000 + statutory_maternity_pay: 2_000 + child: + age: 5 + benunits: + benunit: + members: [parent, child] + households: + household: + members: [parent, child] + tenure_type: RENT_FROM_COUNCIL + rent: 9_600 + output: + income_tax: [1_886, 0] + uc_income_tax_on_earnings: [1_886, 0] + uc_national_insurance_on_earnings: [754.40, 0] + uc_individual_earned_income: [19_359.60, 0] + uc_earned_income: 14_235.60 + universal_credit: 10_516.50 + +- name: The same total paid as wages gives the same award + period: 2026 + absolute_error_margin: 0.01 + input: + people: + parent: + age: 30 + employment_income: 22_000 + child: + age: 5 + benunits: + benunit: + members: [parent, child] + households: + household: + members: [parent, child] + tenure_type: RENT_FROM_COUNCIL + rent: 9_600 + output: + uc_individual_earned_income: [19_359.60, 0] + uc_earned_income: 14_235.60 + universal_credit: 10_516.50 + +- name: Statutory sick pay alone is earned income + period: 2026 + absolute_error_margin: 0.01 + input: + age: 30 + statutory_sick_pay: 3_000 + output: + # Below the personal allowance and the primary threshold. + uc_income_tax_on_earnings: 0 + uc_national_insurance_on_earnings: 0 + uc_individual_earned_income: 3_000 + # No work allowance: no child and no limited capability for work. + uc_earned_income: 3_000 + # 424.90 x 12. + uc_maximum_amount: 5_098.80 + # 5,098.80 - 0.55 x 3,000. Before this fix the full 5,098.80. + universal_credit: 3_448.80 + +- name: Joint claimants' statutory paternity and maternity pay are combined earned income + period: 2026 + absolute_error_margin: 0.01 + input: + people: + father: + age: 30 + employment_income: 9_000 + statutory_paternity_pay: 400 + mother: + age: 30 + statutory_maternity_pay: 6_000 + child: + age: 1 + benunits: + benunit: + members: [father, mother, child] + households: + household: + members: [father, mother, child] + tenure_type: RENT_FROM_COUNCIL + rent: 9_600 + output: + # Each is below the personal allowance and the primary threshold, so + # nothing is deducted: 9,000 + 400 and 6,000. + uc_income_tax_on_earnings: [0, 0, 0] + uc_national_insurance_on_earnings: [0, 0, 0] + uc_individual_earned_income: [9_400, 6_000, 0] + # 15,400 less the work allowance of 5,124. + uc_earned_income: 10_276 + # 666.97 x 12 + 303.94 x 12 + 9,600 = 8,003.64 + 3,647.28 + 9,600. + uc_maximum_amount: 21_250.92 + # 21,250.92 - 0.55 x 10,276. Before this fix only the 9,000 of wages + # counted: 21,250.92 - 0.55 x 3,876 = 19,119.12. + universal_credit: 15_599.12 + +- name: National Insurance on statutory paternity pay comes off the earnings that include it + period: 2026 + absolute_error_margin: 0.01 + input: + age: 30 + employment_income: 12_000 + statutory_paternity_pay: 400 + output: + # (12,400 - 12,569.96) is negative, so no National Insurance, and wages + # of 12,000 are below the personal allowance. + uc_national_insurance_on_earnings: 0 + uc_income_tax_on_earnings: 0 + uc_individual_earned_income: 12_400 + # 5,098.80 - 0.55 x 12,400 is negative. + universal_credit: 0 + +- name: A qualifying young person's statutory sick pay is not the claimant's earned income + # Reg. 22(1)(b) deducts "the claimant's earned income (or, in the case of + # joint claimants, their combined earned income)". + period: 2026 + absolute_error_margin: 0.01 + input: + people: + parent: + age: 40 + is_parent: true + young_person: + age: 17 + current_education: UPPER_SECONDARY + statutory_sick_pay: 2_000 + benunits: + benunit: + members: [parent, young_person] + households: + household: + members: [parent, young_person] + output: + uc_individual_earned_income_before_mif: [0, 2_000] + uc_earned_income: 0 + # 424.90 x 12 + 351.88 x 12, the higher child element for a first child + # born before 6 April 2017. The sick pay takes nothing off it. + universal_credit: 9_321.36 diff --git a/policyengine_uk/tests/test_uc_statutory_pay_properties.py b/policyengine_uk/tests/test_uc_statutory_pay_properties.py new file mode 100644 index 0000000000..f1f84036c1 --- /dev/null +++ b/policyengine_uk/tests/test_uc_statutory_pay_properties.py @@ -0,0 +1,497 @@ +"""Property-based tests that Universal Credit counts statutory pay as earnings. + +Universal Credit Regulations 2013 reg. 55(4): statutory sick pay, statutory +maternity pay, statutory paternity pay, statutory adoption pay, statutory +shared parental pay, statutory parental bereavement pay and statutory neonatal +care pay "are to be treated as employed earnings". Reg. 55(5) takes relievable +pension contributions, income tax and primary Class 1 contributions off "the +amount of general earnings or benefits specified in paragraphs (2) to (4)". +The model has variables for the first three payments. It charges National +Insurance on all three (ni_class_1_income) and income tax on sick and maternity +pay (employment_benefits in taxable_employment_income), as it does on wages. + +Before this was fixed the payments were left out of earned income while the +tax and National Insurance on them were still deducted, so the award rose by +the taper rate times that tax as a payment rose. + +Invariants, for any generated population of single claimants and couples, with +or without children, in which one member receives one of the three payments: + +1. Monotonic: raising a claimant's payment, with everything else fixed, never + raises the award. Two rules of law make exceptions, and neither is a fault: + + a. Reg. 32(2)(b) treats a claimant receiving statutory sick, maternity or + paternity pay as in paid work for the childcare costs element. So the + first pound of a payment can open that element for a claimant with no + other paid work. The award then rises by no more than the element does, + and only as the payment starts from nil + (test_first_pound_of_statutory_pay_can_open_the_childcare_element). + b. Income Tax Act 2007 s. 55B(2)(b) withdraws the Marriage Allowance tax + reduction in full once the recipient is liable at a rate above the + basic rate. A pound of pay that crosses that line raises the tax on the + earnings by more than a pound, earned income falls and the award rises. + Wages do the same + (test_marriage_allowance_cliff_is_the_same_as_for_wages), so the + monotonic property is stated for families that do not claim Marriage + Allowance. + +2. Same as wages: statutory sick or maternity pay of s on top of wages of e + gives the same income tax, National Insurance, earned income and award + before the benefit cap as wages of e + s. This holds for Marriage Allowance + claimants too. Two differences in the model's income tax are left out + (counts_as_wages): it charges no income tax on statutory paternity pay, and + it caps pension contribution relief at employment and self-employment + income without statutory pay. The award after the benefit cap is not + compared, because the cap's earnings exception + (is_benefit_cap_exempt_earnings) reads employment income without statutory + pay. + +3. Accounting: a person's earned income before the minimum income floor is + their wages, statutory pay and trading profit (a loss counts as nil) less + their relievable pension contributions and the income tax and National + Insurance on their earnings, floored at nil. + +4. Bounds: a pound more of a payment adds between nothing and a pound to the + recipient's earned income, for families that do not claim Marriage + Allowance. + +5. A dependant's payment changes neither the claimants' earned income nor the + award (reg. 22(1)(b): "the claimant's earned income (or, in the case of + joint claimants, their combined earned income)"). + +Every member's is_claimant_or_partner is set from its generated role, as the +FRS supplies it, and every attribute is set for every member: setting an +input for anyone makes it an input for everyone. +""" + +import numpy as np +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st + +from policyengine_uk import Simulation + +YEAR = 2026 +PROPERTY_SETTINGS = settings( + max_examples=10, + deadline=None, + derandomize=True, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.data_too_large], +) +# Reg. 55(4) payments the model has variables for. +PAYMENTS = [ + "statutory_sick_pay", + "statutory_maternity_pay", + "statutory_paternity_pay", +] +# The model charges income tax on these, as on wages (employment_benefits). +TAXED_AS_WAGES = ["statutory_sick_pay", "statutory_maternity_pay"] +# Wages and payments on and around the 2026-27 thresholds: the personal +# allowance and primary threshold (12,570), the basic rate limit (50,270) and +# the personal allowance taper (100,000 to 125,140). +WAGES = [0, 0, 6_000, 12_570, 16_000, 24_000, 50_000, 50_270, 62_000, 100_000, 126_000] +PAY = [0, 0, 1, 300, 2_000, 6_000, 12_570, 20_000, 40_000] +# Float32 awards of tens of thousands of pounds are exact to about a penny. +TOLERANCE = 0.05 + +# The three versions of each family that a simulation holds. +LOW, HIGH, AS_WAGES = range(3) +VERSIONS = [LOW, HIGH, AS_WAGES] + + +@st.composite +def adults(draw, marriage_allowance): + return dict( + age=draw(st.sampled_from([22, 30, 45, 62, 68])), + wages=float(draw(st.sampled_from(WAGES))), + profit=float(draw(st.sampled_from([0, 0, 0, 9_000, 30_000, -4_000]))), + startup=draw(st.booleans()), + pension_contributions=float(draw(st.sampled_from([0, 0, 2_000]))), + pension_income=float(draw(st.sampled_from([0, 0, 6_000]))), + dividends=float(draw(st.sampled_from([0, 0, 4_000]))), + lcwra=draw(st.booleans()), + claims_marriage_allowance=marriage_allowance and draw(st.booleans()), + ) + + +@st.composite +def families(draw, marriage_allowance=False, dependant_recipient=False): + members = draw(st.lists(adults(marriage_allowance), min_size=1, max_size=2)) + child_ages = draw(st.lists(st.sampled_from([1, 6, 13]), max_size=3)) + if dependant_recipient: + # A qualifying young person: 17 and in non-advanced education. + child_ages = [17] + child_ages[:2] + recipient = len(members) + else: + recipient = draw(st.integers(0, len(members) - 1)) + amounts = st.one_of(st.sampled_from(PAY), st.integers(0, 40_000)) + low, high = sorted([draw(amounts), draw(amounts)]) + return dict( + adults=members, + child_ages=child_ages, + childcare=float(draw(st.sampled_from([0, 0, 6_000]))), + region=draw( + st.sampled_from( + ["NORTH_EAST", "LONDON", "SCOTLAND", "WALES", "NORTHERN_IRELAND"] + ) + ), + tenure=draw(st.sampled_from(["RENT_FROM_COUNCIL", "RENT_PRIVATELY"])), + rent=float(draw(st.sampled_from([0, 7_200, 18_000, 36_000]))), + recipient=recipient, + payment=draw(st.sampled_from(PAYMENTS)), + low=float(low), + high=float(high), + ) + + +def populations(**kwargs): + # Several families share one simulation, which costs little more than + # one family does. + return st.lists(families(**kwargs), min_size=4, max_size=10) + + +def member_inputs(family, index, version): + """Inputs for one member of one version of a family. + + LOW and HIGH pay the recipient the lower and higher amount of the + payment. AS_WAGES pays the higher amount as wages instead. + """ + is_adult = index < len(family["adults"]) + is_recipient = index == family["recipient"] + paid = {LOW: family["low"], HIGH: family["high"], AS_WAGES: 0.0}[version] + extra_wages = family["high"] if is_recipient and version == AS_WAGES else 0.0 + if is_adult: + adult = family["adults"][index] + inputs = { + "age": adult["age"], + "is_claimant_or_partner": True, + "employment_income": adult["wages"] + extra_wages, + "self_employment_income": adult["profit"], + "uc_is_in_startup_period": adult["startup"], + "employee_pension_contributions": adult["pension_contributions"], + "private_pension_income": adult["pension_income"], + "dividend_income": adult["dividends"], + "uc_limited_capability_for_WRA": adult["lcwra"], + "would_claim_marriage_allowance": adult["claims_marriage_allowance"], + "childcare_expenses": 0.0, + "current_education": "NOT_IN_EDUCATION", + } + else: + age = family["child_ages"][index - len(family["adults"])] + inputs = { + "age": age, + "is_claimant_or_partner": False, + "employment_income": extra_wages, + "self_employment_income": 0.0, + "uc_is_in_startup_period": False, + "employee_pension_contributions": 0.0, + "private_pension_income": 0.0, + "dividend_income": 0.0, + "uc_limited_capability_for_WRA": False, + "would_claim_marriage_allowance": False, + "childcare_expenses": family["childcare"] if age < 16 else 0.0, + "current_education": "UPPER_SECONDARY" if age >= 16 else "PRIMARY", + } + for payment in PAYMENTS: + inputs[payment] = paid if is_recipient and payment == family["payment"] else 0.0 + return {name: {YEAR: value} for name, value in inputs.items()} + + +class Results: + """One simulation holding every family once per version.""" + + def __init__(self, population): + people, benunits, households = {}, {}, {} + self.first_member = {} + for i, family in enumerate(population): + size = len(family["adults"]) + len(family["child_ages"]) + for version in VERSIONS: + self.first_member[i, version] = len(people) + names = [f"person_{i}_{version}_{j}" for j in range(size)] + for j, name in enumerate(names): + people[name] = member_inputs(family, j, version) + benunits[f"benunit_{i}_{version}"] = {"members": names} + households[f"household_{i}_{version}"] = { + "members": names, + "region": {YEAR: family["region"]}, + "tenure_type": {YEAR: family["tenure"]}, + "rent": {YEAR: family["rent"]}, + } + self.population = population + self.simulation = Simulation( + situation=dict(people=people, benunits=benunits, households=households) + ) + + def families(self, variable, version): + """A benefit unit variable for one version of every family.""" + values = np.array(self.simulation.calculate(variable, YEAR), dtype=float) + return values.reshape(len(self.population), len(VERSIONS))[:, version] + + def person(self, variable, i, version, member): + values = np.array(self.simulation.calculate(variable, YEAR), dtype=float) + return values[self.first_member[i, version] + member] + + def recipient(self, variable, i, version): + return self.person(variable, i, version, self.population[i]["recipient"]) + + +@PROPERTY_SETTINGS +@given(population=populations()) +def test_award_never_rises_with_a_claimants_statutory_pay(population): + results = Results(population) + award = {v: results.families("universal_credit", v) for v in (LOW, HIGH)} + childcare = {v: results.families("uc_childcare_element", v) for v in (LOW, HIGH)} + earned = "uc_individual_earned_income_before_mif" + for i, family in enumerate(population): + if family["low"] > 0: + # The payment is already in payment, so reg. 32(2)(b) changes + # nothing: the award cannot rise at all. + assert childcare[HIGH][i] == childcare[LOW][i], family + assert award[HIGH][i] <= award[LOW][i] + TOLERANCE, family + else: + # The payment starts. It can open the childcare costs element + # (reg. 32(2)(b)) and nothing else. + opened = max(0, childcare[HIGH][i] - childcare[LOW][i]) + assert award[HIGH][i] <= award[LOW][i] + opened + TOLERANCE, family + # Invariant 4: a pound more adds between nothing and a pound to the + # recipient's earned income. + gain = results.recipient(earned, i, HIGH) - results.recipient(earned, i, LOW) + assert gain >= -TOLERANCE, family + assert gain <= family["high"] - family["low"] + TOLERANCE, family + + +def counts_as_wages(family): + """Whether the model taxes the family's payment exactly as it taxes wages. + + It charges no income tax on statutory paternity pay. And + pension_contributions_relief caps relief at employment and self-employment + income, which leaves statutory pay out: a recipient whose wages and profit + are below their pension contributions gets less relief on statutory pay + than on the same amount of wages. + """ + if family["recipient"] >= len(family["adults"]): + return False + recipient = family["adults"][family["recipient"]] + covered = ( + recipient["wages"] + recipient["profit"] >= recipient["pension_contributions"] + ) + return family["payment"] in TAXED_AS_WAGES and covered + + +@PROPERTY_SETTINGS +@given(population=populations(marriage_allowance=True)) +def test_statutory_pay_counts_as_wages_and_earned_income_adds_up(population): + results = Results(population) + for i, family in enumerate(population): + # Invariant 3, for every adult. No adult is 75 or over, the age from + # which contributions stop being relievable. + for member, adult in enumerate(family["adults"]): + paid = family["high"] if member == family["recipient"] else 0.0 + gross = adult["wages"] + paid + max(0, adult["profit"]) + deductions = adult["pension_contributions"] + sum( + results.person(variable, i, HIGH, member) + for variable in [ + "uc_income_tax_on_earnings", + "uc_national_insurance_on_earnings", + ] + ) + np.testing.assert_allclose( + results.person( + "uc_individual_earned_income_before_mif", i, HIGH, member + ), + max(0, gross - deductions), + atol=TOLERANCE, + rtol=1e-6, + err_msg=str(family), + ) + # Invariant 2. + if not counts_as_wages(family): + continue + for variable in [ + "income_tax", + "national_insurance", + "uc_income_tax_on_earnings", + "uc_national_insurance_on_earnings", + "uc_individual_earned_income_before_mif", + "uc_individual_earned_income", + ]: + np.testing.assert_allclose( + results.recipient(variable, i, HIGH), + results.recipient(variable, i, AS_WAGES), + atol=TOLERANCE, + err_msg=f"{variable}: {family}", + ) + for variable in ["uc_earned_income", "universal_credit_pre_benefit_cap"]: + taxed = [counts_as_wages(family) for family in population] + np.testing.assert_allclose( + results.families(variable, HIGH)[taxed], + results.families(variable, AS_WAGES)[taxed], + atol=TOLERANCE, + err_msg=variable, + ) + + +@PROPERTY_SETTINGS +@given(population=populations(marriage_allowance=True, dependant_recipient=True)) +def test_a_dependants_statutory_pay_is_not_the_claimants_earned_income(population): + results = Results(population) + for variable in ["uc_earned_income", "universal_credit"]: + np.testing.assert_array_equal( + results.families(variable, HIGH), + results.families(variable, LOW), + err_msg=variable, + ) + # The young person does have the earnings; they are just not the + # claimants'. + earned = "uc_individual_earned_income_before_mif" + for i, family in enumerate(population): + assert ( + results.recipient(earned, i, HIGH) + >= results.recipient(earned, i, LOW) - TOLERANCE + ), family + + +# Deterministic paths through the 2026-27 thresholds, for each payment. +PATH = [ + 0, + 1, + 100, + 269, + 270, + 271, + 1_000, + 2_000, + 5_000, + 12_569, + 12_570, + 12_571, + 20_000, + 37_700, + 50_270, + 50_271, + 80_000, +] + + +def adult(age=30, wages=0.0, profit=0.0, claims_marriage_allowance=False): + return dict( + age=age, + wages=wages, + profit=profit, + startup=False, + pension_contributions=0.0, + pension_income=0.0, + dividends=0.0, + lcwra=False, + claims_marriage_allowance=claims_marriage_allowance, + ) + + +def family(adults, child_ages=(), rent=0.0, region="NORTH_EAST", childcare=0.0): + return dict( + adults=adults, + child_ages=list(child_ages), + childcare=childcare, + region=region, + tenure="RENT_FROM_COUNCIL", + rent=rent, + recipient=0, + ) + + +ARCHETYPES = { + "single adult with no other income": family([adult()]), + "lone parent on 20,000": family([adult(wages=20_000)], [5], rent=9_600), + "couple on 50,000, crossing the basic rate limit": family( + [adult(40, wages=50_000), adult(38)], [5, 5, 5], rent=36_000 + ), + "couple on 99,000, crossing the personal allowance taper": family( + [adult(40, wages=99_000), adult(38)], [5, 5, 5], rent=36_000 + ), + "Scottish lone parent on 30,000": family( + [adult(wages=30_000)], [5, 5], rent=18_000, region="SCOTLAND" + ), + "self-employed below the minimum income floor": family( + [adult(profit=5_000)], rent=9_600 + ), +} + + +def along_paths(bases, payments, variables): + """Each variable along PATH, for each base family and payment. + + Returns {variable: {version: array[base, payment, step]}}, where HIGH + raises the payment along the path and AS_WAGES raises wages instead. + """ + population = [ + dict(base, payment=payment, low=0.0, high=float(step)) + for base in bases + for payment in payments + for step in PATH + ] + results = Results(population) + shape = (len(bases), len(payments), len(PATH)) + return { + variable: { + version: results.families(variable, version).reshape(shape) + for version in (HIGH, AS_WAGES) + } + for variable in variables + } + + +def test_award_falls_along_a_path_of_rising_statutory_pay(): + paths = along_paths(list(ARCHETYPES.values()), PAYMENTS, ["universal_credit"]) + awards = paths["universal_credit"][HIGH] + for i, name in enumerate(ARCHETYPES): + for j, payment in enumerate(PAYMENTS): + path = awards[i, j] + assert path[0] > 0, name + assert (np.diff(path) <= TOLERANCE).all(), (name, payment, path) + # The path takes the award down, so the test can fail. + assert path[-1] < path[0] - 1_000, (name, payment, path) + + +def test_first_pound_of_statutory_pay_can_open_the_childcare_element(): + # Intended: reg. 32(2)(b) treats a claimant receiving statutory sick, + # maternity or paternity pay as in paid work, which the childcare costs + # element requires (reg. 32(1)(a)). The family has no rent, so the + # benefit cap does not take any of the element back. + base = family([adult()], [5], childcare=6_000) + paths = along_paths([base], PAYMENTS, ["universal_credit", "uc_childcare_element"]) + for j, payment in enumerate(PAYMENTS): + awards = paths["universal_credit"][HIGH][0, j] + childcare = paths["uc_childcare_element"][HIGH][0, j] + assert childcare[0] == 0 + assert childcare[1] > 0 + # The award rises by the element, less at most the pound of pay. + rise = awards[1] - awards[0] + assert childcare[1] - 1 - TOLERANCE <= rise <= childcare[1] + TOLERANCE + # Once the payment is in payment the element does not move, and the + # award never rises again. + assert (childcare[1:] == childcare[1]).all() + assert (np.diff(awards[1:]) <= TOLERANCE).all(), (payment, awards) + + +def test_marriage_allowance_cliff_is_the_same_as_for_wages(): + # Intended: ITA 2007 s. 55B(2)(b) withdraws the whole Marriage Allowance + # reduction once the recipient is liable above the basic rate. The pound + # of pay that takes the recipient past the basic rate limit therefore + # lowers earned income and raises the award, exactly as a pound of wages + # does. The monotonic property above leaves these families out. + base = family( + [ + adult(40, wages=50_000, claims_marriage_allowance=True), + adult(38, claims_marriage_allowance=True), + ], + [5, 5, 5], + rent=36_000, + ) + variables = ["uc_earned_income", "universal_credit_pre_benefit_cap"] + paths = along_paths([base], TAXED_AS_WAGES, variables) + for variable in variables: + np.testing.assert_allclose( + paths[variable][HIGH], + paths[variable][AS_WAGES], + atol=TOLERANCE, + err_msg=variable, + ) diff --git a/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_individual_earned_income_before_mif.py b/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_individual_earned_income_before_mif.py index 73a93702a1..94cd2ba2a0 100644 --- a/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_individual_earned_income_before_mif.py +++ b/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_individual_earned_income_before_mif.py @@ -9,16 +9,19 @@ class uc_individual_earned_income_before_mif(Variable): ) documentation = ( "The person's actual earned income for Universal Credit: their " - "earnings less their own relievable pension contributions and their " - "own income tax and National Insurance in respect of their employment " + "earnings, including statutory sick, maternity and paternity pay, " + "less their own relievable pension contributions and their own " + "income tax and National Insurance in respect of their employment " "and self-employment. The minimum income floor compares this with " - "the net threshold." + "the net threshold. Regulation 55(4) also treats statutory adoption, " + "shared parental, parental bereavement and neonatal care pay as " + "employed earnings; the model has no variables for those." ) definition_period = YEAR unit = GBP reference = [ dict( - title="Universal Credit Regulations 2013 reg. 55(5)", + title="Universal Credit Regulations 2013 reg. 55(4) and (5)", href="https://www.legislation.gov.uk/uksi/2013/376/regulation/55", ), dict( @@ -35,7 +38,20 @@ def formula(person, period, parameters): # The deductions are the person's own and come off only their own # earnings, so one partner's tax, NI or pension contributions never # reduce the other partner's earned income. - earnings_components = ["employment_income", "miscellaneous_income"] + # Reg. 55(4) treats statutory sick, maternity and paternity pay as + # employed earnings, and reg. 55(5) takes the deductions off "the + # amount of general earnings or benefits specified in paragraphs (2) + # to (4)". Income tax and National Insurance already fall on these + # payments (taxable_employment_income, ni_class_1_income), so leaving + # them out of the earnings deducted the tax on them from other + # earnings and raised the award as the payments rose. + earnings_components = [ + "employment_income", + "miscellaneous_income", + "statutory_sick_pay", + "statutory_maternity_pay", + "statutory_paternity_pay", + ] bi = parameters(period).gov.contrib.ubi_center.basic_income if bi.interactions.include_in_means_tests: earnings_components.append("basic_income") From b0e8fb17967475684675fe9609c957cba9e18490 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 21:01:39 -0400 Subject: [PATCH 16/27] Count Scottish Child Payment, Healthy Start and the Two Child Limit Payment in government spending HOUSEHOLD_BENEFIT_VARIABLES listed scottish_child_payment, healthy_start_vouchers and two_child_limit_payment, but GOV_SPENDING_VARIABLES did not, so households received them at no cost to the government and gov_balance was overstated by the amount paid. two_child_limit_payment is zero at baseline (the regulations were not laid) but is paid when a reform turns it on. Adds a code-health test that every household benefit is in government spending or on an explicit, documented NOT_PAID_BY_GOVERNMENT list (empty today), YAML cases, and Hypothesis properties: gov_spending is the sum of its list, gov_balance = gov_tax - gov_spending, household benefits equal government spending on households, and a pound more of each payment is a pound more spending and a pound less balance. Fixes #2338 Co-Authored-By: Claude Opus 5.5 --- changelog.d/2338.fixed.md | 1 + ...usehold_benefits_in_government_spending.py | 86 +++++++ .../policy/baseline/gov/gov_spending.yaml | 75 ++++++ .../tests/test_gov_spending_properties.py | 221 ++++++++++++++++++ policyengine_uk/variables/gov/gov_spending.py | 3 + 5 files changed, 386 insertions(+) create mode 100644 changelog.d/2338.fixed.md create mode 100644 policyengine_uk/tests/code_health/test_household_benefits_in_government_spending.py create mode 100644 policyengine_uk/tests/policy/baseline/gov/gov_spending.yaml create mode 100644 policyengine_uk/tests/test_gov_spending_properties.py diff --git a/changelog.d/2338.fixed.md b/changelog.d/2338.fixed.md new file mode 100644 index 0000000000..ec98dcdd0c --- /dev/null +++ b/changelog.d/2338.fixed.md @@ -0,0 +1 @@ +- Counted the Scottish Child Payment, Healthy Start vouchers and the Two Child Limit Payment in government spending, so `gov_balance` no longer leaves out benefits that households receive. diff --git a/policyengine_uk/tests/code_health/test_household_benefits_in_government_spending.py b/policyengine_uk/tests/code_health/test_household_benefits_in_government_spending.py new file mode 100644 index 0000000000..e4a60cea73 --- /dev/null +++ b/policyengine_uk/tests/code_health/test_household_benefits_in_government_spending.py @@ -0,0 +1,86 @@ +"""Every benefit that government pays a household is government spending. + +household_benefits adds HOUSEHOLD_BENEFIT_VARIABLES to household net income. +gov_balance subtracts gov_spending, which adds GOV_SPENDING_VARIABLES. A +benefit in the first list but not the second raises household income at no +cost to the government, so gov_balance overstates the public finances by the +amount paid. The Scottish Child Payment, Healthy Start vouchers and the Two +Child Limit Payment were left out in that way. + +Invariant: every entry of HOUSEHOLD_BENEFIT_VARIABLES is in +GOV_SPENDING_VARIABLES, or is in NOT_PAID_BY_GOVERNMENT below with the reason +government does not pay it. +""" + +from policyengine_uk import CountryTaxBenefitSystem +from policyengine_uk.variables.gov.gov_spending import GOV_SPENDING_VARIABLES +from policyengine_uk.variables.household.income.household_benefits import ( + HOUSEHOLD_BENEFIT_VARIABLES, +) + +SYSTEM = CountryTaxBenefitSystem() + +# Household benefits that government does not pay, each with the reason. +# gov_spending leaves these out. Empty: government pays every benefit in +# HOUSEHOLD_BENEFIT_VARIABLES. Add an entry only for a benefit that someone +# other than government funds; a benefit government pays goes in +# GOV_SPENDING_VARIABLES instead. +NOT_PAID_BY_GOVERNMENT = {} + +# The benefits this invariant found missing from government spending. +ADDED_TO_GOVERNMENT_SPENDING = [ + "healthy_start_vouchers", + "scottish_child_payment", + "two_child_limit_payment", +] + + +def left_out_of_government_spending(benefits, spending, exclusions): + """Household benefits that are neither government spending nor excluded.""" + return sorted(set(benefits) - set(spending) - set(exclusions)) + + +def test_every_household_benefit_is_government_spending_or_excluded(): + left_out = left_out_of_government_spending( + HOUSEHOLD_BENEFIT_VARIABLES, GOV_SPENDING_VARIABLES, NOT_PAID_BY_GOVERNMENT + ) + assert not left_out, ( + f"household_benefits counts {left_out} but gov_spending does not. Add " + "each to GOV_SPENDING_VARIABLES, or to NOT_PAID_BY_GOVERNMENT with the " + "reason government does not pay it." + ) + + +def test_exclusions_are_household_benefits_outside_government_spending(): + for name, reason in NOT_PAID_BY_GOVERNMENT.items(): + assert name in SYSTEM.variables, f"{name} is not a variable" + assert name in HOUSEHOLD_BENEFIT_VARIABLES, ( + f"{name} is excluded but is not a household benefit" + ) + assert name not in GOV_SPENDING_VARIABLES, ( + f"{name} is excluded as not paid by government but is in gov_spending" + ) + assert isinstance(reason, str) and reason.strip(), ( + f"{name} needs a reason government does not pay it" + ) + + +def test_the_check_reports_a_benefit_left_out_of_government_spending(): + # The lists as they were before the three benefits were added. + before = [ + name + for name in GOV_SPENDING_VARIABLES + if name not in ADDED_TO_GOVERNMENT_SPENDING + ] + assert left_out_of_government_spending( + HOUSEHOLD_BENEFIT_VARIABLES, before, NOT_PAID_BY_GOVERNMENT + ) == sorted(ADDED_TO_GOVERNMENT_SPENDING) + # An exclusion with a reason takes a benefit out of the report. + assert ( + left_out_of_government_spending( + ["paid_by_government", "paid_by_employer"], + ["paid_by_government"], + {"paid_by_employer": "the employer pays it"}, + ) + == [] + ) diff --git a/policyengine_uk/tests/policy/baseline/gov/gov_spending.yaml b/policyengine_uk/tests/policy/baseline/gov/gov_spending.yaml new file mode 100644 index 0000000000..850d997186 --- /dev/null +++ b/policyengine_uk/tests/policy/baseline/gov/gov_spending.yaml @@ -0,0 +1,75 @@ +# Government spending counts every benefit that government pays a household. +# The Scottish Child Payment, Healthy Start vouchers and the Two Child Limit +# Payment were in household_benefits but not in gov_spending, so gov_balance +# left out their cost. +# +# 2026-27 rates: UC standard allowance 424.90 a month (single, 25 or over) and +# child element 303.94 a month; Child Benefit 27.05 a week for the eldest +# child; Scottish Child Payment 28.20 a week. + +- name: Scottish Child Payment and Healthy Start vouchers are government spending + period: 2026 + absolute_error_margin: 0.01 + input: + people: + parent: + age: 30 + child: + age: 3 + # An amount the household reports; the model does not calculate it. + healthy_start_vouchers: 221 + benunits: + benunit: + members: [parent, child] + households: + household: + members: [parent, child] + region: SCOTLAND + output: + # 28.20 x 52. + scottish_child_payment: [0, 1_466.40] + # 424.90 x 12 + 303.94 x 12. + universal_credit: 8_746.08 + # 27.05 x 52. + child_benefit: 1_406.60 + # 8,746.08 + 1,406.60 + 1,466.40 + 221. + household_benefits: 11_840.08 + # The same four payments. Before this fix 10,152.68: without the + # Scottish Child Payment and the vouchers. + gov_spending: 11_840.08 + +- name: The Two Child Limit Payment is government spending when a reform turns it on + period: 2025 + absolute_error_margin: 0.01 + input: + gov.social_security_scotland.two_child_limit_payment.in_effect: true + people: + parent: + age: 30 + child_1: + age: 5 + uc_is_child_limit_affected: false + child_2: + age: 3 + uc_is_child_limit_affected: false + child_3: + age: 1 + uc_is_child_limit_affected: true + benunits: + benunit: + members: [parent, child_1, child_2, child_3] + # Entered directly, so that only the payments under test are computed. + universal_credit: 0 + child_benefit: 0 + households: + household: + members: [parent, child_1, child_2, child_3] + region: SCOTLAND + output: + # The UC child element for the one affected child: 292.81 x 12. + two_child_limit_payment: 3_513.72 + # No Universal Credit, so no Scottish Child Payment. + scottish_child_payment: [0, 0, 0, 0] + household_benefits: 3_513.72 + # Before this fix 0. + gov_spending: 3_513.72 diff --git a/policyengine_uk/tests/test_gov_spending_properties.py b/policyengine_uk/tests/test_gov_spending_properties.py new file mode 100644 index 0000000000..dea15731bb --- /dev/null +++ b/policyengine_uk/tests/test_gov_spending_properties.py @@ -0,0 +1,221 @@ +"""Property-based tests that government spending counts household benefits. + +gov_spending adds every entry of GOV_SPENDING_VARIABLES and gov_balance is +gov_tax less gov_spending. household_benefits adds every entry of +HOUSEHOLD_BENEFIT_VARIABLES to household net income. The Scottish Child +Payment, Healthy Start vouchers and the Two Child Limit Payment were in the +second list but not the first, so households received them at no cost to the +government and gov_balance was overstated by the amount paid. + +Invariants, for any generated population of households: + +1. Sum identity: gov_spending equals the sum, over the distinct names in + GOV_SPENDING_VARIABLES, of each variable summed to the household. +2. Balance identity: gov_balance equals gov_tax less gov_spending. +3. Conservation: what households receive in benefits, less any benefit that + government does not pay (NOT_PAID_BY_GOVERNMENT), equals gov_spending less + the spending that is not a household benefit. So a pound of benefit from + government is a pound of government spending. +4. Metamorphic: paying a household d more of the Scottish Child Payment, + Healthy Start vouchers or the Two Child Limit Payment raises + household_benefits and gov_spending by d each, lowers gov_balance by d and + leaves gov_tax unchanged. + +Invariants 3 and 4 fail on the earlier list whenever a household receives one +of the three payments. + +The three payments are entered directly for every member, so the properties +test the lists and not the payments' own eligibility rules. +""" + +import numpy as np +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st + +from policyengine_uk import Simulation +from policyengine_uk.tests.code_health.test_household_benefits_in_government_spending import ( + NOT_PAID_BY_GOVERNMENT, +) +from policyengine_uk.variables.gov.gov_spending import GOV_SPENDING_VARIABLES +from policyengine_uk.variables.household.income.household_benefits import ( + HOUSEHOLD_BENEFIT_VARIABLES, +) + +YEAR = 2026 +PROPERTY_SETTINGS = settings( + max_examples=10, + deadline=None, + derandomize=True, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.data_too_large], +) +SPENDING = list(dict.fromkeys(GOV_SPENDING_VARIABLES)) +BENEFITS = list(dict.fromkeys(HOUSEHOLD_BENEFIT_VARIABLES)) +# Spending that is not a benefit a household receives. +NOT_HOUSEHOLD_BENEFITS = [name for name in SPENDING if name not in BENEFITS] +# The payments that were missing from government spending, with the entity +# each is paid to. +PAYMENTS = { + "scottish_child_payment": "child", + "healthy_start_vouchers": "child", + "two_child_limit_payment": "benunit", +} +REGIONS = ["SCOTLAND", "NORTH_EAST", "LONDON", "WALES", "NORTHERN_IRELAND"] +TENURES = ["RENT_FROM_COUNCIL", "RENT_PRIVATELY", "OWNED_OUTRIGHT"] +# Float32 household totals of tens of thousands of pounds are exact to about +# a penny. +TOLERANCE = 0.05 + +money = st.floats(0, 60_000, allow_nan=False, allow_infinity=False) +payment = st.one_of(st.just(0.0), st.floats(1, 4_000, allow_nan=False)) + + +@st.composite +def households(draw): + return dict( + adults=[ + dict( + age=draw(st.integers(18, 80)), + employment_income=draw(st.one_of(st.just(0.0), money)), + ) + for _ in range(draw(st.integers(1, 2))) + ], + children=[ + dict( + age=draw(st.integers(0, 15)), + scottish_child_payment=draw(payment), + healthy_start_vouchers=draw(payment), + ) + for _ in range(draw(st.integers(0, 3))) + ], + two_child_limit_payment=draw(payment), + would_claim_uc=draw(st.booleans()), + region=draw(st.sampled_from(REGIONS)), + tenure=draw(st.sampled_from(TENURES)), + rent=draw(st.floats(0, 15_000, allow_nan=False)), + ) + + +def situation(units, bumped=None, bump=0.0, start=0): + """The households, paying `bump` more of the `bumped` payment. + + A child payment rises for every child; the benefit unit payment rises + once. Households are numbered from `start`. + """ + people, benunits, homes = {}, {}, {} + for i, unit in enumerate(units, start): + names = [] + for j, adult in enumerate(unit["adults"]): + name = f"a{i}_{j}" + people[name] = { + "age": {YEAR: adult["age"]}, + "employment_income": {YEAR: adult["employment_income"]}, + "scottish_child_payment": {YEAR: 0.0}, + "healthy_start_vouchers": {YEAR: 0.0}, + } + names.append(name) + for j, child in enumerate(unit["children"]): + name = f"c{i}_{j}" + people[name] = { + "age": {YEAR: child["age"]}, + "employment_income": {YEAR: 0.0}, + } + for paid in ["scottish_child_payment", "healthy_start_vouchers"]: + people[name][paid] = { + YEAR: child[paid] + (bump if paid == bumped else 0.0) + } + names.append(name) + benunits[f"b{i}"] = { + "members": names, + "would_claim_uc": {YEAR: unit["would_claim_uc"]}, + "two_child_limit_payment": { + YEAR: unit["two_child_limit_payment"] + + (bump if bumped == "two_child_limit_payment" else 0.0) + }, + } + homes[f"h{i}"] = { + "members": names, + "region": {YEAR: unit["region"]}, + "tenure_type": {YEAR: unit["tenure"]}, + "rent": {YEAR: unit["rent"]}, + } + return {"people": people, "benunits": benunits, "households": homes} + + +def calc(sim, name): + return np.asarray(sim.calculate(name, YEAR, map_to="household"), dtype=float) + + +def household_sum(sim, names): + return sum((calc(sim, name) for name in names), np.zeros(1)) + + +@PROPERTY_SETTINGS +@given(st.lists(households(), min_size=1, max_size=12)) +def test_government_spending_counts_every_benefit_government_pays(units): + sim = Simulation(situation=situation(units)) + spending = calc(sim, "gov_spending") + # Invariant 1. + np.testing.assert_allclose( + spending, household_sum(sim, SPENDING), atol=TOLERANCE, rtol=1e-6 + ) + # Invariant 2. + np.testing.assert_allclose( + calc(sim, "gov_balance"), + calc(sim, "gov_tax") - spending, + atol=TOLERANCE, + rtol=1e-6, + ) + # Invariant 3. + np.testing.assert_allclose( + calc(sim, "household_benefits") + - household_sum(sim, list(NOT_PAID_BY_GOVERNMENT)), + spending - household_sum(sim, NOT_HOUSEHOLD_BENEFITS), + atol=TOLERANCE, + rtol=1e-6, + ) + # The payments under test are in the population, so invariant 3 can fail. + entered = sum( + child[name] + for unit in units + for child in unit["children"] + for name in ["scottish_child_payment", "healthy_start_vouchers"] + ) + sum(unit["two_child_limit_payment"] for unit in units) + np.testing.assert_allclose( + household_sum(sim, list(PAYMENTS)).sum(), entered, rtol=1e-5, atol=TOLERANCE + ) + + +@PROPERTY_SETTINGS +@given( + st.lists(households(), min_size=1, max_size=12), + st.sampled_from(list(PAYMENTS)), + st.floats(1, 3_000, allow_nan=False), +) +def test_a_pound_more_of_a_payment_is_a_pound_of_government_spending( + units, bumped, bump +): + # One simulation holds every household twice: as drawn, then with the + # extra payment. + as_drawn = situation(units) + with_bump = situation(units, bumped=bumped, bump=bump, start=len(units)) + sim = Simulation( + situation={ + entity: {**as_drawn[entity], **with_bump[entity]} for entity in as_drawn + } + ) + recipients = np.array( + [len(unit["children"]) if PAYMENTS[bumped] == "child" else 1 for unit in units] + ) + paid = recipients * bump + + def change(name): + values = calc(sim, name) + return values[len(units) :] - values[: len(units)] + + np.testing.assert_allclose(change(bumped), paid, rtol=1e-5, atol=TOLERANCE) + np.testing.assert_allclose( + change("household_benefits"), paid, rtol=1e-5, atol=TOLERANCE + ) + np.testing.assert_allclose(change("gov_spending"), paid, rtol=1e-5, atol=TOLERANCE) + np.testing.assert_allclose(change("gov_tax"), 0, atol=TOLERANCE) + np.testing.assert_allclose(change("gov_balance"), -paid, rtol=1e-5, atol=TOLERANCE) diff --git a/policyengine_uk/variables/gov/gov_spending.py b/policyengine_uk/variables/gov/gov_spending.py index ca27cf2a72..211a6f8d21 100644 --- a/policyengine_uk/variables/gov/gov_spending.py +++ b/policyengine_uk/variables/gov/gov_spending.py @@ -49,6 +49,9 @@ "dfe_education_spending", "dft_subsidy_spending", "nhs_spending", + "healthy_start_vouchers", + "two_child_limit_payment", + "scottish_child_payment", "carer_support_payment", "scottish_carer_supplement", "disability_basic_income", From 0dce50c86c0fcb0e139797f03cb1b4f29be45d38 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 22:22:29 -0400 Subject: [PATCH 17/27] Count paternity pay as household income; state the benefit cap exception Review of 70851abb1 (GPT-6.1 Sol) found two blocking problems: - Statutory paternity pay was in no household income list, while statutory sick and maternity pay were. Once Universal Credit tapers it, household net income fell as paternity pay rose (UC -220 on 400 of pay, with the pay never counted). Add it next to maternity pay in household_benefits, both HBAI lists and gov_spending, and add a net income regression. - The monotonic property checked the award after the benefit cap. Reg. 82(1)(a) lifts the cap once earned income reaches the threshold, and statutory pay is earned income, so a correct cap would fail it. Check the award before the cap, and let the final award rise only where the family becomes exempt. Also: prove the Marriage Allowance cliff directly, qualify it for Scottish taxpayers, add a property that each payment counts once in household_benefits, add a YAML case where NI on paternity pay is deducted, and correct the comment and changelog: income tax falls on sick and maternity pay only. Co-Authored-By: Claude Opus 5.5 --- changelog.d/2337.fixed.md | 2 +- .../income/uc_statutory_pay.yaml | 55 ++++++- .../tests/test_uc_statutory_pay_properties.py | 153 +++++++++++++++--- .../uc_individual_earned_income_before_mif.py | 10 +- policyengine_uk/variables/gov/gov_spending.py | 1 + .../household/income/hbai_benefits.py | 1 + .../income/hbai_household_net_income.py | 1 + .../household/income/household_benefits.py | 1 + 8 files changed, 194 insertions(+), 30 deletions(-) diff --git a/changelog.d/2337.fixed.md b/changelog.d/2337.fixed.md index 17d3f13dbf..7d8bb217b2 100644 --- a/changelog.d/2337.fixed.md +++ b/changelog.d/2337.fixed.md @@ -1 +1 @@ -- Counted statutory sick, maternity and paternity pay as Universal Credit earned income (UC Regs 2013 reg. 55(4)). The income tax and National Insurance on them were already deducted from earned income, so Universal Credit rose as the payments rose. +- Counted statutory sick, maternity and paternity pay as Universal Credit earned income (UC Regs 2013 reg. 55(4)). Universal Credit already deducted the National Insurance on all three, and the income tax on sick and maternity pay, so it rose as the payments rose. Statutory paternity pay is now also counted in household benefits, HBAI income and government spending, as sick and maternity pay already were. diff --git a/policyengine_uk/tests/policy/baseline/finance/benefit/family/universal_credit/income/uc_statutory_pay.yaml b/policyengine_uk/tests/policy/baseline/finance/benefit/family/universal_credit/income/uc_statutory_pay.yaml index 910c62df7d..c7c8c2d29f 100644 --- a/policyengine_uk/tests/policy/baseline/finance/benefit/family/universal_credit/income/uc_statutory_pay.yaml +++ b/policyengine_uk/tests/policy/baseline/finance/benefit/family/universal_credit/income/uc_statutory_pay.yaml @@ -7,8 +7,9 @@ # bereavement and neonatal care pay too; the model has no variables for them. # # Before this was fixed the payments were left out of earned income while the -# income tax and National Insurance on them were still deducted. Each pound of -# sick or maternity pay above the thresholds then raised the award by +# National Insurance on them, and the income tax on sick and maternity pay, +# were still deducted (the model charges no income tax on paternity pay). Each +# pound of sick or maternity pay above the thresholds then raised the award by # 0.55 x (20% + 8%) = 15.4p; each pound of paternity pay by 0.55 x 8% = 4.4p. # # Hand calculations use the 2026-27 rates: @@ -201,3 +202,53 @@ # 424.90 x 12 + 351.88 x 12, the higher child element for a first child # born before 6 April 2017. The sick pay takes nothing off it. universal_credit: 9_321.36 + +- name: National Insurance on statutory paternity pay above the threshold comes off earned income + # Employee pension contributions of 8,000 under a net pay arrangement take + # taxable income below the personal allowance, so no income tax is due + # whether or not paternity pay is taxed; National Insurance is charged on + # pay before pension contributions. + period: 2026 + absolute_error_margin: 0.01 + input: + age: 30 + employment_income: 20_000 + employee_pension_contributions: 8_000 + statutory_paternity_pay: 400 + output: + uc_income_tax_on_earnings: 0 + # (20,000 + 400 - 12,569.96) x 8%. + uc_national_insurance_on_earnings: 626.40 + # 20,400 - 8,000 relievable pension contributions - 626.40 (reg. 55(5)). + uc_individual_earned_income: 11_773.60 + +- name: Statutory paternity pay is household income, so it never lowers net income through Universal Credit + # Statutory sick and maternity pay were already in household benefits, + # HBAI income and government spending; paternity pay was in none of them. + # Once Universal Credit tapers it, leaving it out would make net income + # fall by 0.55 x 400 = 220 as the pay rose. + period: 2026 + absolute_error_margin: 0.01 + input: + people: + parent: + age: 30 + statutory_paternity_pay: 400 + benunits: + benunit: + members: [parent] + households: + household: + members: [parent] + council_tax: 0 + tv_licence: 0 + expected_sdlt: 0 + output: + # 424.90 x 12 - 0.55 x 400, with no work allowance. + universal_credit: 4_878.80 + # The award plus the pay. Without the pay the household has 5,098.80, so + # 400 of pay raises its income by 180. + household_benefits: 5_278.80 + household_net_income: 5_278.80 + hbai_household_net_income: 5_278.80 + gov_spending: 5_278.80 diff --git a/policyengine_uk/tests/test_uc_statutory_pay_properties.py b/policyengine_uk/tests/test_uc_statutory_pay_properties.py index f1f84036c1..ef2de488c3 100644 --- a/policyengine_uk/tests/test_uc_statutory_pay_properties.py +++ b/policyengine_uk/tests/test_uc_statutory_pay_properties.py @@ -11,14 +11,16 @@ pay (employment_benefits in taxable_employment_income), as it does on wages. Before this was fixed the payments were left out of earned income while the -tax and National Insurance on them were still deducted, so the award rose by -the taper rate times that tax as a payment rose. +National Insurance on them, and the income tax on sick and maternity pay, were +still deducted, so the award rose by the taper rate times that tax as a +payment rose. Invariants, for any generated population of single claimants and couples, with or without children, in which one member receives one of the three payments: 1. Monotonic: raising a claimant's payment, with everything else fixed, never - raises the award. Two rules of law make exceptions, and neither is a fault: + raises the award before the benefit cap. Three rules of law make + exceptions, and none is a fault: a. Reg. 32(2)(b) treats a claimant receiving statutory sick, maternity or paternity pay as in paid work for the childcare costs element. So the @@ -27,13 +29,21 @@ and only as the payment starts from nil (test_first_pound_of_statutory_pay_can_open_the_childcare_element). b. Income Tax Act 2007 s. 55B(2)(b) withdraws the Marriage Allowance tax - reduction in full once the recipient is liable at a rate above the - basic rate. A pound of pay that crosses that line raises the tax on the - earnings by more than a pound, earned income falls and the award rises. - Wages do the same + reduction in full once the recipient is liable at a rate the subsection + does not list (it lists the basic rate and, for a Scottish taxpayer, the + Scottish rates up to the intermediate rate). A pound of pay that crosses + that line raises the tax on the earnings by more than a pound, earned + income falls and the award rises. Wages do the same (test_marriage_allowance_cliff_is_the_same_as_for_wages), so the monotonic property is stated for families that do not claim Marriage Allowance. + c. The benefit cap does not apply where earned income reaches the reg. + 82(1)(a) threshold, and the payments are earned income (reg. 55(4)). So + a pound of pay can lift the cap and raise the final award. The award + after the cap may therefore rise only where the family becomes exempt + (is_benefit_cap_exempt); otherwise it obeys the same bound. Today's + is_benefit_cap_exempt_earnings reads employment income without + statutory pay, so this exception does not yet arise in the model. 2. Same as wages: statutory sick or maternity pay of s on top of wages of e gives the same income tax, National Insurance, earned income and award @@ -55,6 +65,12 @@ recipient's earned income, for families that do not claim Marriage Allowance. +6. Household income: each payment counts once in household_benefits, and so + in household net income. Raising it by d raises household_benefits by d + plus the change in the other benefits listed there. Statutory paternity + pay was in no household income list, so once Universal Credit tapered it + household net income fell as the pay rose. + 5. A dependant's payment changes neither the claimants' earned income nor the award (reg. 22(1)(b): "the claimant's earned income (or, in the case of joint claimants, their combined earned income)"). @@ -68,7 +84,14 @@ from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st -from policyengine_uk import Simulation +from policyengine_uk import CountryTaxBenefitSystem, Simulation +from policyengine_uk.variables.gov.gov_spending import GOV_SPENDING_VARIABLES +from policyengine_uk.variables.household.income.hbai_household_net_income import ( + HBAI_HOUSEHOLD_NET_INCOME_ADDS, +) +from policyengine_uk.variables.household.income.household_benefits import ( + HOUSEHOLD_BENEFIT_VARIABLES, +) YEAR = 2026 PROPERTY_SETTINGS = settings( @@ -85,6 +108,7 @@ ] # The model charges income tax on these, as on wages (employment_benefits). TAXED_AS_WAGES = ["statutory_sick_pay", "statutory_maternity_pay"] +BENEFITS = list(dict.fromkeys(HOUSEHOLD_BENEFIT_VARIABLES)) # Wages and payments on and around the 2026-27 thresholds: the personal # allowance and primary threshold (12,570), the basic rate limit (50,270) and # the personal allowance taper (100,000 to 125,140). @@ -226,6 +250,13 @@ def families(self, variable, version): values = np.array(self.simulation.calculate(variable, YEAR), dtype=float) return values.reshape(len(self.population), len(VERSIONS))[:, version] + def households(self, variable, version): + """Any variable summed to the household, for one version of every family.""" + values = np.array( + self.simulation.calculate(variable, YEAR, map_to="household"), dtype=float + ) + return values.reshape(len(self.population), len(VERSIONS))[:, version] + def person(self, variable, i, version, member): values = np.array(self.simulation.calculate(variable, YEAR), dtype=float) return values[self.first_member[i, version] + member] @@ -238,25 +269,66 @@ def recipient(self, variable, i, version): @given(population=populations()) def test_award_never_rises_with_a_claimants_statutory_pay(population): results = Results(population) - award = {v: results.families("universal_credit", v) for v in (LOW, HIGH)} - childcare = {v: results.families("uc_childcare_element", v) for v in (LOW, HIGH)} + + def both(variable): + return {v: results.families(variable, v) for v in (LOW, HIGH)} + + before_cap = both("universal_credit_pre_benefit_cap") + award = both("universal_credit") + childcare = both("uc_childcare_element") + exempt = both("is_benefit_cap_exempt") earned = "uc_individual_earned_income_before_mif" for i, family in enumerate(population): if family["low"] > 0: # The payment is already in payment, so reg. 32(2)(b) changes - # nothing: the award cannot rise at all. + # nothing. assert childcare[HIGH][i] == childcare[LOW][i], family - assert award[HIGH][i] <= award[LOW][i] + TOLERANCE, family - else: - # The payment starts. It can open the childcare costs element - # (reg. 32(2)(b)) and nothing else. - opened = max(0, childcare[HIGH][i] - childcare[LOW][i]) + # Exception a: a payment that starts can open the childcare costs + # element, and nothing else. + opened = max(0, childcare[HIGH][i] - childcare[LOW][i]) + assert before_cap[HIGH][i] <= before_cap[LOW][i] + opened + TOLERANCE, family + # Exception c: more earned income never ends an exemption from the + # cap, and only a new exemption can raise the award after the cap. + assert exempt[HIGH][i] >= exempt[LOW][i], family + if exempt[HIGH][i] == exempt[LOW][i]: assert award[HIGH][i] <= award[LOW][i] + opened + TOLERANCE, family # Invariant 4: a pound more adds between nothing and a pound to the # recipient's earned income. gain = results.recipient(earned, i, HIGH) - results.recipient(earned, i, LOW) assert gain >= -TOLERANCE, family assert gain <= family["high"] - family["low"] + TOLERANCE, family + # Invariant 6: each payment is in household_benefits once. + for payment in PAYMENTS: + rows = np.array([family["payment"] == payment for family in population]) + if not rows.any(): + continue + + def change(variable): + return results.households(variable, HIGH) - results.households( + variable, LOW + ) + + others = sum(change(benefit) for benefit in BENEFITS if benefit != payment) + paid = np.array([family["high"] - family["low"] for family in population]) + np.testing.assert_allclose( + (change("household_benefits") - others)[rows], + paid[rows], + atol=TOLERANCE, + rtol=1e-5, + err_msg=payment, + ) + + +def test_each_payment_is_household_income_and_government_spending(): + lists = { + "HOUSEHOLD_BENEFIT_VARIABLES": HOUSEHOLD_BENEFIT_VARIABLES, + "HBAI_HOUSEHOLD_NET_INCOME_ADDS": HBAI_HOUSEHOLD_NET_INCOME_ADDS, + "hbai_benefits.adds": CountryTaxBenefitSystem().variables["hbai_benefits"].adds, + "GOV_SPENDING_VARIABLES": GOV_SPENDING_VARIABLES, + } + for name, listed in lists.items(): + for payment in PAYMENTS: + assert listed.count(payment) == 1, (name, payment) def counts_as_wages(family): @@ -440,15 +512,25 @@ def along_paths(bases, payments, variables): def test_award_falls_along_a_path_of_rising_statutory_pay(): - paths = along_paths(list(ARCHETYPES.values()), PAYMENTS, ["universal_credit"]) - awards = paths["universal_credit"][HIGH] + variables = [ + "universal_credit_pre_benefit_cap", + "universal_credit", + "is_benefit_cap_exempt", + ] + paths = along_paths(list(ARCHETYPES.values()), PAYMENTS, variables) for i, name in enumerate(ARCHETYPES): for j, payment in enumerate(PAYMENTS): - path = awards[i, j] + path = paths["universal_credit_pre_benefit_cap"][HIGH][i, j] assert path[0] > 0, name assert (np.diff(path) <= TOLERANCE).all(), (name, payment, path) # The path takes the award down, so the test can fail. assert path[-1] < path[0] - 1_000, (name, payment, path) + # After the cap, the award rises only at a step that makes the + # family exempt (reg. 82(1)(a)). + final = paths["universal_credit"][HIGH][i, j] + newly_exempt = np.diff(paths["is_benefit_cap_exempt"][HIGH][i, j]) > 0 + rises = np.diff(final) > TOLERANCE + assert not (rises & ~newly_exempt).any(), (name, payment, final) def test_first_pound_of_statutory_pay_can_open_the_childcare_element(): @@ -474,10 +556,11 @@ def test_first_pound_of_statutory_pay_can_open_the_childcare_element(): def test_marriage_allowance_cliff_is_the_same_as_for_wages(): # Intended: ITA 2007 s. 55B(2)(b) withdraws the whole Marriage Allowance - # reduction once the recipient is liable above the basic rate. The pound - # of pay that takes the recipient past the basic rate limit therefore - # lowers earned income and raises the award, exactly as a pound of wages - # does. The monotonic property above leaves these families out. + # reduction once the recipient (an English taxpayer here) is liable above + # the basic rate. The pound of pay that takes the recipient past the + # basic rate limit therefore lowers earned income and raises the award, + # exactly as a pound of wages does. The monotonic property above leaves + # these families out. base = family( [ adult(40, wages=50_000, claims_marriage_allowance=True), @@ -495,3 +578,27 @@ def test_marriage_allowance_cliff_is_the_same_as_for_wages(): atol=TOLERANCE, err_msg=variable, ) + # The cliff is there: the reduction goes in one step, and at that step + # earned income falls and the award rises. + population = [ + dict(base, payment=payment, low=0.0, high=float(step)) + for payment in TAXED_AS_WAGES + for step in PATH + ] + results = Results(population) + shape = (len(TAXED_AS_WAGES), len(PATH)) + reduction = np.array( + [ + results.recipient("marriage_allowance", i, HIGH) + for i in range(len(population)) + ] + ).reshape(shape) + for j, payment in enumerate(TAXED_AS_WAGES): + assert reduction[j, 0] > 0 and reduction[j, -1] == 0, (payment, reduction[j]) + steps = np.flatnonzero(np.diff(reduction[j]) < 0) + assert len(steps) == 1, (payment, reduction[j]) + step = steps[0] + earned = paths["uc_earned_income"][HIGH][0, j] + award = paths["universal_credit_pre_benefit_cap"][HIGH][0, j] + assert earned[step + 1] < earned[step], (payment, earned) + assert award[step + 1] > award[step], (payment, award) diff --git a/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_individual_earned_income_before_mif.py b/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_individual_earned_income_before_mif.py index 94cd2ba2a0..75d6669a3b 100644 --- a/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_individual_earned_income_before_mif.py +++ b/policyengine_uk/variables/gov/dwp/universal_credit/income/uc_individual_earned_income_before_mif.py @@ -41,10 +41,12 @@ def formula(person, period, parameters): # Reg. 55(4) treats statutory sick, maternity and paternity pay as # employed earnings, and reg. 55(5) takes the deductions off "the # amount of general earnings or benefits specified in paragraphs (2) - # to (4)". Income tax and National Insurance already fall on these - # payments (taxable_employment_income, ni_class_1_income), so leaving - # them out of the earnings deducted the tax on them from other - # earnings and raised the award as the payments rose. + # to (4)". National Insurance falls on all three (ni_class_1_income) + # and income tax on sick and maternity pay (employment_benefits in + # taxable_employment_income), and both are deducted below. Leaving + # the payments out of the earnings deducted the tax and National + # Insurance on them from other earnings, so the award rose as the + # payments rose. earnings_components = [ "employment_income", "miscellaneous_income", diff --git a/policyengine_uk/variables/gov/gov_spending.py b/policyengine_uk/variables/gov/gov_spending.py index ca27cf2a72..bc8c8e6e53 100644 --- a/policyengine_uk/variables/gov/gov_spending.py +++ b/policyengine_uk/variables/gov/gov_spending.py @@ -27,6 +27,7 @@ "maternity_allowance", "statutory_sick_pay", "statutory_maternity_pay", + "statutory_paternity_pay", "ssmg", "basic_income", "epg_subsidy", diff --git a/policyengine_uk/variables/household/income/hbai_benefits.py b/policyengine_uk/variables/household/income/hbai_benefits.py index ac8d146133..010155a879 100644 --- a/policyengine_uk/variables/household/income/hbai_benefits.py +++ b/policyengine_uk/variables/household/income/hbai_benefits.py @@ -32,6 +32,7 @@ class hbai_benefits(Variable): "maternity_allowance", "statutory_sick_pay", "statutory_maternity_pay", + "statutory_paternity_pay", "ssmg", "cost_of_living_support_payment", "winter_fuel_allowance", diff --git a/policyengine_uk/variables/household/income/hbai_household_net_income.py b/policyengine_uk/variables/household/income/hbai_household_net_income.py index 5061ab23c1..936ebea59a 100644 --- a/policyengine_uk/variables/household/income/hbai_household_net_income.py +++ b/policyengine_uk/variables/household/income/hbai_household_net_income.py @@ -42,6 +42,7 @@ "maternity_allowance", "statutory_sick_pay", "statutory_maternity_pay", + "statutory_paternity_pay", "ssmg", "cost_of_living_support_payment", "winter_fuel_allowance", diff --git a/policyengine_uk/variables/household/income/household_benefits.py b/policyengine_uk/variables/household/income/household_benefits.py index 1de62cbaec..81c86e6072 100644 --- a/policyengine_uk/variables/household/income/household_benefits.py +++ b/policyengine_uk/variables/household/income/household_benefits.py @@ -27,6 +27,7 @@ "maternity_allowance", "statutory_sick_pay", "statutory_maternity_pay", + "statutory_paternity_pay", "ssmg", "basic_income", "epg_subsidy", From 175ac4b85d2293f2ffcb40de4c40453a7005a348 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 23:15:49 -0400 Subject: [PATCH 18/27] Roll back failed override batches and keep structural-reform nodes across rebuilds Fixes #2374, the low-severity notes from GPT-6.1 Sol's round-3 review of #2305: - add_legacy_overrides applies a batch to a copy and swaps it in, so a batch that fails applies none of its overrides. - Rebuilds (apply_parameter_assignments) and reprocessing (reset_parameters then process_parameters) add back the nodes a structural reform added to the processed parameters (graft_missing_nodes), so their overrides replay; process_parameters rolls back on failure. - The shared projected timelines are documented as shared. - The scenario docs say Scenario.apply edits the annualised values, and which steps run after the UC rebalancing and structural reforms. Co-Authored-By: Claude Opus 5.5 --- changelog.d/2374.fixed.md | 1 + docs/book/usage/scenarios.md | 4 +- policyengine_uk/tax_benefit_system.py | 65 +++++++++--- .../tests/test_dated_reform_routes.py | 99 +++++++++++++++++++ policyengine_uk/utils/dated_reforms.py | 24 +++++ 5 files changed, 179 insertions(+), 14 deletions(-) create mode 100644 changelog.d/2374.fixed.md diff --git a/changelog.d/2374.fixed.md b/changelog.d/2374.fixed.md new file mode 100644 index 0000000000..1d71ba5221 --- /dev/null +++ b/changelog.d/2374.fixed.md @@ -0,0 +1 @@ +- A failed batch of legacy parameter overrides is now rolled back whole, nodes a structural reform adds to the parameters survive later rebuilds and reprocessing, and the scenario docs say that `Scenario.apply` on a built simulation edits the annualised values. diff --git a/docs/book/usage/scenarios.md b/docs/book/usage/scenarios.md index ffc01ede7b..d50098a3f0 100644 --- a/docs/book/usage/scenarios.md +++ b/docs/book/usage/scenarios.md @@ -396,7 +396,9 @@ Each way of passing a reform reads period keys as it always has: A single-date key in a `reform=` dict overrides the annualised value on that day only. Formulas read annual values on 1 January, so `"2027-01-01"` sets fiscal year 2027 and `"2027-04-06"` changes nothing. The model logs a warning when a key can change nothing. Use `"start.stop"` keys for dated measures. -Changes applied after the data is loaded (the default) leave the data as built, so a changed economic assumption does not re-uprate incomes. They still come before the steps that read parameters while the simulation is built: the Universal Credit rebalancing awards and the structural reforms that parameters switch on. Set `applied_before_data_load=True` on a scenario to project the data with changed economic assumptions too. +Changes applied after the data is loaded (the default) leave the data as built, so a changed economic assumption does not re-uprate incomes. Dated changes (`parameter_changes` and `"start.stop"` or bare-year keys) still come before the steps that read parameters while the simulation is built: the Universal Credit rebalancing awards and the structural reforms that parameters switch on. Single-day keys and a scenario's own simulation modifier run after those steps, as they always have. Set `applied_before_data_load=True` on a scenario to project the data with changed economic assumptions too. + +All of this applies when the scenario or reform is passed to `Simulation(...)`. `Scenario.apply(simulation)` on a simulation that is already built edits its annualised values instead, as it always has: a `"2027-04-06.2028-04-05"` key there overrides those dates of the fiscal-year values, so it does not set fiscal year 2027, and the model logs a warning saying so. Pass the scenario when constructing the simulation for dated measures. ### Creating complex scenarios with simulation modifiers diff --git a/policyengine_uk/tax_benefit_system.py b/policyengine_uk/tax_benefit_system.py index 9e4175a690..f0a2cb3ebc 100644 --- a/policyengine_uk/tax_benefit_system.py +++ b/policyengine_uk/tax_benefit_system.py @@ -53,6 +53,7 @@ Timeline, apply_legacy_overrides, copy_timelines, + graft_missing_nodes, requires_projection, snapshot_timelines, tree_defines, @@ -173,8 +174,14 @@ def reset_parameter_caches(self): self.parameters._at_instant_cache = {} def reset_parameters(self) -> None: - """Reset parameters by reloading from the parameters directory.""" + """Reset parameters by reloading from the parameters directory. + + The processed parameters it replaces are kept until + ``process_parameters`` runs, so nodes a structural reform added to + them survive the reload. + """ self._parameters_at_instant_cache = {} + self._parameters_before_reset = getattr(self, "parameters", None) self.load_parameters(self.parameters_dir) self.parameters = add_removed_parameter_aliases(self.parameters) @@ -188,14 +195,31 @@ def process_parameters(self) -> None: The system's dated reform changes (``dated_assignments``) are written onto the parameters first, so a modifier that reloads and edits the parameters before processing them again (as ``no_economic_assumptions`` - does) keeps the reform. Its legacy overrides of the annualised - parameters are then applied again. The parameters as they stood - before the changes are kept as the base for later rebuilds. + does) keeps the reform. Nodes a structural reform added to the + processed parameters before ``reset_parameters`` are added back, and + the legacy overrides of the annualised parameters are applied again. + The parameters as they stood before the changes are kept as the base + for later rebuilds. If processing fails, the system is left as it + was. """ + previous = (self.parameters, self._projected_timelines, self._raw_parameters) + before_reset = getattr(self, "_parameters_before_reset", None) self._parameters_at_instant_cache = {} - self._rebuild(self.parameters, statutory=False) - self._apply_legacy_overrides() - self.reset_parameter_caches() + try: + self._rebuild(self.parameters, statutory=False) + if before_reset is not None: + graft_missing_nodes(before_reset, self.parameters) + self._apply_legacy_overrides() + except Exception: + ( + self.parameters, + self._projected_timelines, + self._raw_parameters, + ) = previous + raise + finally: + self.reset_parameter_caches() + self._parameters_before_reset = None def apply_parameter_assignments( self, assignments: List[ParameterAssignment] @@ -213,10 +237,10 @@ def apply_parameter_assignments( parameter takes under the reform's other changes (see ``policyengine_uk.utils.dated_reforms``). - A rebuild keeps the system's dated changes and legacy overrides. It - does not keep other edits made to the processed parameters, such as - a structural reform's, as ``reset_parameters`` never has. If the - rebuild fails, the system is left as it was. + A rebuild keeps the system's dated changes and legacy overrides, and + the nodes a structural reform added to the processed parameters. It + does not keep other edits made directly to existing processed + parameters. If the rebuild fails, the system is left as it was. """ if not assignments: return @@ -240,6 +264,8 @@ def apply_parameter_assignments( cached = _rebuilt_parameters_cache.get(key) if key is not None else None if cached is not None: self.parameters = cached[0].clone() + # Shared with the memo: read only through the copying + # ``projected_timelines`` property. self._projected_timelines = cached[1] else: raw = ( @@ -254,6 +280,7 @@ def apply_parameter_assignments( self.parameters.clone(), copy_timelines(self._projected_timelines), ) + graft_missing_nodes(previous[1], self.parameters) self._apply_legacy_overrides() except Exception: ( @@ -270,12 +297,17 @@ def add_legacy_overrides(self, overrides: List[LegacyOverride]) -> None: """Apply changes to the annualised parameters and keep them. Kept so a later rebuild of the parameters applies them again, after - the dated changes. Overrides that fail to apply are not kept. + the dated changes. If any override fails, none is applied or kept. """ if not overrides: return + previous = self.parameters + self.parameters = previous.clone() try: apply_legacy_overrides(self.parameters, overrides) + except Exception: + self.parameters = previous + raise finally: self.reset_parameter_caches() self.legacy_overrides = self.legacy_overrides + list(overrides) @@ -352,6 +384,9 @@ def __init__(self): # when it has been processed itself (None: current law's). self._raw_parameters: Optional[ParameterNode] = None self._projected_timelines: Optional[Dict[str, Timeline]] = None + # The processed parameters ``reset_parameters`` replaced, until + # ``process_parameters`` adds back the nodes only they had. + self._parameters_before_reset: Optional[ParameterNode] = None # Set up and process parameters self.parameters_dir = COUNTRY_DIR / "parameters" @@ -400,7 +435,11 @@ def statutory_parameter_defines() -> Callable[[str], bool]: def projected_timelines() -> Dict[str, Timeline]: - """Current law's projected timeline for each gov parameter, by name.""" + """Current law's projected timeline for each gov parameter, by name. + + Shared: do not change them. ``CountryTaxBenefitSystem.projected_timelines`` + returns a copy. + """ _ensure_caches() return _projected_timelines_cache diff --git a/policyengine_uk/tests/test_dated_reform_routes.py b/policyengine_uk/tests/test_dated_reform_routes.py index 383231dfe6..14cbc83f12 100644 --- a/policyengine_uk/tests/test_dated_reform_routes.py +++ b/policyengine_uk/tests/test_dated_reform_routes.py @@ -1024,3 +1024,102 @@ def test_preserve_calendar_dates_changes_keep_their_dates(start, length, value): while day < datetime.date(2035, 1, 1): assert parameter(day.isoformat()) == timeline_value(expected, day), day day += datetime.timedelta(days=37) + + +# Review round 3 (GPT-6.1 Sol): failed override batches roll back, and +# nodes a structural reform adds survive rebuilds and reprocessing. + + +class _add_bonus(Reform): + def apply(self): + from policyengine_core.parameters import ParameterNode + + self.parameters.add_child( + "test_budget", + ParameterNode("test_budget", data={"bonus": {"values": {"2000-01-01": 1}}}), + ) + + +def test_a_failed_override_batch_applies_none_of_its_overrides(): + simulation = Simulation(situation=SITUATION) + system = simulation.tax_benefit_system + with pytest.raises(ValueError): + Scenario.from_reform( + {BASIC_RATE: {"2027": 0.3}, "gov.not_a_real_parameter": {"2027": 1}} + ).apply(simulation) + assert annual(simulation, BASIC_RATE, 2027) == 0.20 + assert system.legacy_overrides == [] + simulation.apply_parameter_changes({UC_SINGLE_OLD: {"2027": 500}}) + assert annual(simulation, BASIC_RATE, 2027) == 0.20 + assert annual(simulation, UC_SINGLE_OLD, 2027) == 500 + + +def test_a_rebuild_keeps_a_structural_reforms_parameter(): + simulation = Simulation( + situation=SITUATION, reform=(_add_bonus, {"test_budget.bonus": 7}) + ) + simulation.apply_parameter_changes({BASIC_RATE: {"2027": 0.21}}) + parameters = simulation.tax_benefit_system.parameters + assert parameters.test_budget.bonus("2027-01-01") == 7 + assert annual(simulation, BASIC_RATE, 2027) == 0.21 + # Repeated rebuilds keep it too. + simulation.apply_parameter_changes({UC_SINGLE_OLD: {"2027": 500}}) + assert simulation.tax_benefit_system.parameters.test_budget.bonus("2027-01-01") == 7 + + +def test_reprocessing_keeps_a_structural_reforms_parameter(): + from policyengine_uk.scenarios.no_economic_assumptions import ( + no_economic_assumptions, + ) + + simulation = Simulation( + situation=SITUATION, reform=(_add_bonus, {"test_budget.bonus": 7}) + ) + no_economic_assumptions.apply(simulation) + parameters = simulation.tax_benefit_system.parameters + assert parameters.test_budget.bonus("2027-01-01") == 7 + assert annual(simulation, PERSONAL_ALLOWANCE, 2035) == 12_570 + assert simulation.tax_benefit_system._parameters_before_reset is None + + +def test_failed_reprocessing_leaves_the_system_as_it_was(monkeypatch): + simulation = Simulation(situation=SITUATION) + Scenario.from_reform({BASIC_RATE: {"2027": 0.3}}).apply(simulation) + system = simulation.tax_benefit_system + system.reset_parameters() + reloaded = (system.parameters, system._projected_timelines, system._raw_parameters) + + # Fail after the rebuild has replaced the parameters, while replaying + # the legacy override. + def fail(*args, **kwargs): + raise RuntimeError("replay failed") + + monkeypatch.setattr(tbs_module, "apply_legacy_overrides", fail) + with pytest.raises(RuntimeError, match="replay failed"): + system.process_parameters() + assert system.parameters is reloaded[0] + assert system._projected_timelines is reloaded[1] + assert system._raw_parameters is reloaded[2] + + +def test_grafting_adds_only_missing_nodes(): + from policyengine_core.parameters import ParameterNode + + from policyengine_uk.utils.dated_reforms import graft_missing_nodes + + source = ParameterNode( + "", + data={ + "a": { + "x": {"values": {"2000-01-01": 1}}, + "y": {"values": {"2000-01-01": 2}}, + }, + "baseline": {"z": {"values": {"2000-01-01": 3}}}, + }, + ) + target = ParameterNode("", data={"a": {"x": {"values": {"2000-01-01": 10}}}}) + added = graft_missing_nodes(source, target) + assert added == ["a.y"] + assert target.a.x("2001-01-01") == 10 + assert target.a.y("2001-01-01") == 2 + assert "baseline" not in target.children diff --git a/policyengine_uk/utils/dated_reforms.py b/policyengine_uk/utils/dated_reforms.py index ae243793f6..69fc09d8c0 100644 --- a/policyengine_uk/utils/dated_reforms.py +++ b/policyengine_uk/utils/dated_reforms.py @@ -502,6 +502,30 @@ def copy_timelines(timelines: Dict[str, Timeline]) -> Dict[str, Timeline]: } +def graft_missing_nodes( + source: ParameterNode, target: ParameterNode, root: bool = True +) -> List[str]: + """Add to ``target`` copies of the nodes ``source`` has and it lacks. + + A rebuild or a reload starts from the YAML, so nodes a structural + reform added to the processed parameters (``parameters.add_child``) + would otherwise be lost. Existing parameters are left as ``target`` has + them, and the root's ``baseline`` child, which every processing builds + again, is skipped. Returns the names of the nodes added. + """ + added = [] + for key, child in source.children.items(): + if root and key == "baseline": + continue + existing = target.children.get(key) + if existing is None: + target.add_child(key, child.clone()) + added.append(child.name) + elif isinstance(child, ParameterNode) and isinstance(existing, ParameterNode): + added += graft_missing_nodes(child, existing, root=False) + return added + + def tree_defines(parameters: ParameterNode) -> Callable[[str], bool]: """``defines(path)``: whether a parameter tree has a leaf at ``path``.""" From 7819755de821b5c9597cf329253f8378f054d145 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 12 Oct 2026 04:58:21 -0400 Subject: [PATCH 19/27] Process a copy on reprocessing, undo override batches in place, and graft scale bands Review of #2375 (GPT-6.1 Sol, review-hardening-out.md): - process_parameters processes a copy of the reloaded tree, so its rollback returns that tree unprocessed, not an object processing had already changed; the test now checks the contents and a retry. - add_legacy_overrides undoes a failed batch by restoring each target's values_list (update replaces it), keeping the tree's identity and costing O(batch) rather than a whole-tree clone. - graft_missing_nodes also appends scale bands a structural reform added and recurses into existing bands. - reset_parameters keeps only a processed tree, and keeps the first one across a second reset before processing. - Docstrings say grafted nodes are copies and come back even if a modifier deleted them after the reset. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tax_benefit_system.py | 31 ++++++--- .../tests/test_dated_reform_routes.py | 69 ++++++++++++++++++- policyengine_uk/utils/dated_reforms.py | 23 +++++-- 3 files changed, 109 insertions(+), 14 deletions(-) diff --git a/policyengine_uk/tax_benefit_system.py b/policyengine_uk/tax_benefit_system.py index f0a2cb3ebc..9647523fcf 100644 --- a/policyengine_uk/tax_benefit_system.py +++ b/policyengine_uk/tax_benefit_system.py @@ -178,10 +178,17 @@ def reset_parameters(self) -> None: The processed parameters it replaces are kept until ``process_parameters`` runs, so nodes a structural reform added to - them survive the reload. + them survive the reload. Only a processed tree is kept, and a second + reset before processing keeps the first one. """ self._parameters_at_instant_cache = {} - self._parameters_before_reset = getattr(self, "parameters", None) + current = getattr(self, "parameters", None) + if ( + getattr(self, "_parameters_before_reset", None) is None + and current is not None + and "baseline" in current.children + ): + self._parameters_before_reset = current self.load_parameters(self.parameters_dir) self.parameters = add_removed_parameter_aliases(self.parameters) @@ -196,8 +203,9 @@ def process_parameters(self) -> None: onto the parameters first, so a modifier that reloads and edits the parameters before processing them again (as ``no_economic_assumptions`` does) keeps the reform. Nodes a structural reform added to the - processed parameters before ``reset_parameters`` are added back, and - the legacy overrides of the annualised parameters are applied again. + processed parameters before ``reset_parameters`` are added back (so + a node a modifier deletes after the reset comes back too), and the + legacy overrides of the annualised parameters are applied again. The parameters as they stood before the changes are kept as the base for later rebuilds. If processing fails, the system is left as it was. @@ -206,7 +214,9 @@ def process_parameters(self) -> None: before_reset = getattr(self, "_parameters_before_reset", None) self._parameters_at_instant_cache = {} try: - self._rebuild(self.parameters, statutory=False) + # Processing changes the tree it is given, so it gets a copy: if + # it fails, the system keeps the tree it had. + self._rebuild(self.parameters.clone(), statutory=False) if before_reset is not None: graft_missing_nodes(before_reset, self.parameters) self._apply_legacy_overrides() @@ -301,12 +311,17 @@ def add_legacy_overrides(self, overrides: List[LegacyOverride]) -> None: """ if not overrides: return - previous = self.parameters - self.parameters = previous.clone() + # Every target is found before anything changes; ``update`` replaces + # a parameter's ``values_list``, so keeping each list is enough to + # undo the batch. + targets = [self.parameters.get_child(override.path) for override in overrides] + saved = [(target, target.values_list) for target in targets] try: apply_legacy_overrides(self.parameters, overrides) except Exception: - self.parameters = previous + for target, values_list in saved: + target.values_list = values_list + target.parent.clear_parent_cache() raise finally: self.reset_parameter_caches() diff --git a/policyengine_uk/tests/test_dated_reform_routes.py b/policyengine_uk/tests/test_dated_reform_routes.py index 14cbc83f12..e8d714dd84 100644 --- a/policyengine_uk/tests/test_dated_reform_routes.py +++ b/policyengine_uk/tests/test_dated_reform_routes.py @@ -1043,14 +1043,20 @@ def apply(self): def test_a_failed_override_batch_applies_none_of_its_overrides(): simulation = Simulation(situation=SITUATION) system = simulation.tax_benefit_system + parameters = system.parameters with pytest.raises(ValueError): Scenario.from_reform( {BASIC_RATE: {"2027": 0.3}, "gov.not_a_real_parameter": {"2027": 1}} ).apply(simulation) assert annual(simulation, BASIC_RATE, 2027) == 0.20 assert system.legacy_overrides == [] + # The batch is undone in place: the tree is the one the system had. + assert system.parameters is parameters + Scenario.from_reform({BASIC_RATE: {"2027": 0.3}}).apply(simulation) + assert system.parameters is parameters + assert annual(simulation, BASIC_RATE, 2027) == 0.3 simulation.apply_parameter_changes({UC_SINGLE_OLD: {"2027": 500}}) - assert annual(simulation, BASIC_RATE, 2027) == 0.20 + assert annual(simulation, BASIC_RATE, 2027) == 0.3 assert annual(simulation, UC_SINGLE_OLD, 2027) == 500 @@ -1100,6 +1106,45 @@ def fail(*args, **kwargs): assert system.parameters is reloaded[0] assert system._projected_timelines is reloaded[1] assert system._raw_parameters is reloaded[2] + # The tree is as reloaded, not processed: no baseline child, and the + # basic rate keeps its legal dates rather than fiscal-year values. + assert "baseline" not in system.parameters.children + # Processing again then works. + monkeypatch.undo() + system.process_parameters() + assert "baseline" in system.parameters.children + assert annual(simulation, BASIC_RATE, 2027) == 0.3 + + +class _add_top_band(Reform): + def apply(self): + scale = self.parameters.gov.hmrc.income_tax.rates.uk + band = scale.brackets[-1].clone() + band.parent = scale + scale.brackets.append(band) + band.threshold.update(period="year:2000:200", value=200_000) + band.rate.update(period="year:2000:200", value=0.5) + + +def test_a_rebuild_keeps_a_band_a_structural_reform_appended(): + simulation = Simulation(situation=SITUATION, reform=_add_top_band) + # The scale has four bands, uk[0] to uk[3]; the reform adds uk[4]. + top = "gov.hmrc.income_tax.rates.uk[4].rate" + assert annual(simulation, top, 2027) == 0.5 + simulation.apply_parameter_changes({BASIC_RATE: {"2027": 0.21}}) + assert annual(simulation, top, 2027) == 0.5 + assert annual(simulation, BASIC_RATE, 2027) == 0.21 + + +def test_a_second_reset_keeps_the_processed_tree_for_grafting(): + simulation = Simulation( + situation=SITUATION, reform=(_add_bonus, {"test_budget.bonus": 7}) + ) + system = simulation.tax_benefit_system + system.reset_parameters() + system.reset_parameters() + system.process_parameters() + assert system.parameters.test_budget.bonus("2027-01-01") == 7 def test_grafting_adds_only_missing_nodes(): @@ -1123,3 +1168,25 @@ def test_grafting_adds_only_missing_nodes(): assert target.a.x("2001-01-01") == 10 assert target.a.y("2001-01-01") == 2 assert "baseline" not in target.children + + +def test_grafting_appends_missing_scale_brackets(): + from policyengine_core.parameters import ParameterNode + + from policyengine_uk.utils.dated_reforms import graft_missing_nodes + + def band(threshold, rate): + return { + "threshold": {"values": {"2000-01-01": threshold}}, + "rate": {"values": {"2000-01-01": rate}}, + } + + source = ParameterNode( + "", data={"s": {"brackets": [band(0, 0.1), band(10, 0.2), band(20, 0.3)]}} + ) + target = ParameterNode("", data={"s": {"brackets": [band(0, 0.15), band(10, 0.2)]}}) + assert graft_missing_nodes(source, target) == ["s[2]"] + assert len(target.s.brackets) == 3 + assert target.s.brackets[0].rate("2001-01-01") == 0.15 + assert target.s.brackets[2].rate("2001-01-01") == 0.3 + assert target.s.brackets[2].parent is target.s diff --git a/policyengine_uk/utils/dated_reforms.py b/policyengine_uk/utils/dated_reforms.py index 69fc09d8c0..12643bfedf 100644 --- a/policyengine_uk/utils/dated_reforms.py +++ b/policyengine_uk/utils/dated_reforms.py @@ -96,7 +96,7 @@ from dataclasses import dataclass from typing import Any, Callable, Dict, Iterable, List, Mapping, Optional, Tuple -from policyengine_core.parameters import Parameter, ParameterNode +from policyengine_core.parameters import Parameter, ParameterNode, ParameterScale from policyengine_core.parameters.parameter_at_instant import ParameterAtInstant from policyengine_core.periods import Instant, Period, instant, period @@ -508,10 +508,12 @@ def graft_missing_nodes( """Add to ``target`` copies of the nodes ``source`` has and it lacks. A rebuild or a reload starts from the YAML, so nodes a structural - reform added to the processed parameters (``parameters.add_child``) - would otherwise be lost. Existing parameters are left as ``target`` has - them, and the root's ``baseline`` child, which every processing builds - again, is skipped. Returns the names of the nodes added. + reform added to the processed parameters (``parameters.add_child``, or + a bracket appended to a scale) would otherwise be lost. Existing + parameters are left as ``target`` has them, and the root's ``baseline`` + child, which every processing builds again, is skipped. Added nodes are + copies of ``source``'s, not processed again. Returns the names of the + nodes added. """ added = [] for key, child in source.children.items(): @@ -523,6 +525,17 @@ def graft_missing_nodes( added.append(child.name) elif isinstance(child, ParameterNode) and isinstance(existing, ParameterNode): added += graft_missing_nodes(child, existing, root=False) + elif isinstance(child, ParameterScale) and isinstance(existing, ParameterScale): + for bracket, existing_bracket in zip(child.brackets, existing.brackets): + added += graft_missing_nodes(bracket, existing_bracket, root=False) + extra = child.brackets[len(existing.brackets) :] + for bracket in extra: + copy = bracket.clone() + copy.parent = existing + existing.brackets.append(copy) + added.append(bracket.name) + if extra: + existing.clear_parent_cache() return added From 09807343489a76225fbd0580303865d6906c9a8a Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 12 Oct 2026 05:10:46 -0400 Subject: [PATCH 20/27] Test Gift Aid's raised limits on the property-first band order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With #2352, savings and dividends sit above property income (ITA 2007 s. 16). Two YAML cases check that the s. 414(2)(b) extension applies on that order: £40,000 of property income with £20,000 of dividends (dividend tax £4,278.75 against £4,528.75) or £20,000 of savings (savings tax £5,646 against £5,846). Co-Authored-By: Claude Opus 5.5 --- .../gov/hmrc/income_tax/gift_aid.yaml | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml index e3ca92829f..39897c8930 100644 --- a/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml +++ b/policyengine_uk/tests/policy/baseline/gov/hmrc/income_tax/gift_aid.yaml @@ -395,3 +395,71 @@ # band remains (£5,270 without), at 18%, and the rest at 24%. capital_gains_tax: [3_703.80, 3_763.80] income_tax: [6_486, 6_486] + +# With #2352, savings and dividends sit above property income (ITA 2007 s. 16), +# and the raised limits must apply on that order. +- name: Gift Aid raises the basic rate limit for dividends above property income + period: 2026 + absolute_error_margin: 0.01 + input: + people: + donor: + age: 40 + property_income: 40_000 + dividend_income: 20_000 + gift_aid: 800 + control: + age: 40 + property_income: 40_000 + dividend_income: 20_000 + benunits: + donor_unit: + members: [donor] + control_unit: + members: [control] + households: + donor_household: + members: [donor] + region: NORTH_WEST + control_household: + members: [control] + region: NORTH_WEST + output: + # Non-savings taxable income £27,430. Without the gift, £500 allowance and + # £9,770 at 10.75% fill the band and £9,730 is at 35.75% (#2352's worked + # example). The gift moves £1,000 from 35.75% to 10.75%: £250. + dividend_income_tax: [4_278.75, 4_528.75] + property_income_tax: [5_486, 5_486] + +- name: Gift Aid raises the basic rate limit for savings above property income + period: 2026 + absolute_error_margin: 0.01 + input: + people: + donor: + age: 40 + property_income: 40_000 + savings_interest_income: 20_000 + gift_aid: 800 + control: + age: 40 + property_income: 40_000 + savings_interest_income: 20_000 + benunits: + donor_unit: + members: [donor] + control_unit: + members: [control] + households: + donor_household: + members: [donor] + region: NORTH_WEST + control_household: + members: [control] + region: NORTH_WEST + output: + # Both still have higher-rate income, so a £500 savings allowance. Without + # the gift, £9,770 at 20% and £9,730 at 40%; the gift moves £1,000 from + # 40% to 20%: £200. + savings_allowance: [500, 500] + savings_income_tax: [5_646, 5_846] From 63d2e7e35ec71a06b573ae89d22a72c17a29433f Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 12 Oct 2026 05:15:55 -0400 Subject: [PATCH 21/27] Restore modified flags when a batch is undone, and describe what rebuilds keep Review round 2 of #2375 (GPT-6.1 Sol, APPROVE with two notes): - a failed override batch now restores the modified flags of its targets and their ancestors as well as their values, so core's cache eligibility is as it was; - the dated_reforms module docstring says rebuilds add back nodes a structural reform added and keep no other edits to existing processed parameters. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tax_benefit_system.py | 13 +++++++++++-- policyengine_uk/tests/test_dated_reform_routes.py | 12 +++++++++++- policyengine_uk/utils/dated_reforms.py | 7 ++++--- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/policyengine_uk/tax_benefit_system.py b/policyengine_uk/tax_benefit_system.py index 9647523fcf..23ef292ec4 100644 --- a/policyengine_uk/tax_benefit_system.py +++ b/policyengine_uk/tax_benefit_system.py @@ -312,16 +312,25 @@ def add_legacy_overrides(self, overrides: List[LegacyOverride]) -> None: if not overrides: return # Every target is found before anything changes; ``update`` replaces - # a parameter's ``values_list``, so keeping each list is enough to - # undo the batch. + # a parameter's ``values_list`` and marks it and its ancestors + # modified, so keeping each list and those flags is enough to undo + # the batch. targets = [self.parameters.get_child(override.path) for override in overrides] saved = [(target, target.values_list) for target in targets] + flags = {} + for target in targets: + node = target + while node is not None and id(node) not in flags: + flags[id(node)] = (node, getattr(node, "modified", False)) + node = getattr(node, "parent", None) try: apply_legacy_overrides(self.parameters, overrides) except Exception: for target, values_list in saved: target.values_list = values_list target.parent.clear_parent_cache() + for node, modified in flags.values(): + node.modified = modified raise finally: self.reset_parameter_caches() diff --git a/policyengine_uk/tests/test_dated_reform_routes.py b/policyengine_uk/tests/test_dated_reform_routes.py index e8d714dd84..d2a2091cb5 100644 --- a/policyengine_uk/tests/test_dated_reform_routes.py +++ b/policyengine_uk/tests/test_dated_reform_routes.py @@ -1044,14 +1044,24 @@ def test_a_failed_override_batch_applies_none_of_its_overrides(): simulation = Simulation(situation=SITUATION) system = simulation.tax_benefit_system parameters = system.parameters + # Processing marks parameters modified, so clear the flags on the path + # to the rate to see whether a failed batch leaves them set. + path = [] + node = parameters.get_child(BASIC_RATE) + while node is not None: + node.modified = False + path.append(node) + node = getattr(node, "parent", None) with pytest.raises(ValueError): Scenario.from_reform( {BASIC_RATE: {"2027": 0.3}, "gov.not_a_real_parameter": {"2027": 1}} ).apply(simulation) assert annual(simulation, BASIC_RATE, 2027) == 0.20 assert system.legacy_overrides == [] - # The batch is undone in place: the tree is the one the system had. + # The batch is undone in place: the tree is the one the system had, and + # nothing on the path to the rate is left marked modified. assert system.parameters is parameters + assert not any(node.modified for node in path) Scenario.from_reform({BASIC_RATE: {"2027": 0.3}}).apply(simulation) assert system.parameters is parameters assert annual(simulation, BASIC_RATE, 2027) == 0.3 diff --git a/policyengine_uk/utils/dated_reforms.py b/policyengine_uk/utils/dated_reforms.py index 12643bfedf..edc9ed837d 100644 --- a/policyengine_uk/utils/dated_reforms.py +++ b/policyengine_uk/utils/dated_reforms.py @@ -79,9 +79,10 @@ been given. Any later rebuild of its parameters (another call to ``Simulation.apply_parameter_changes``, or a modifier that reloads and reprocesses the parameters, as ``no_economic_assumptions`` does) writes them -all again, onto the parameters as that modifier left them. A rebuild does -not keep other edits to the processed parameters, such as a structural -reform's. +all again, onto the parameters as that modifier left them. Nodes a +structural reform added to the processed parameters (a new parameter, or a +band appended to a scale) are added back (``graft_missing_nodes``); other +edits made directly to existing processed parameters are not kept. ``Scenario.apply`` on an already-built simulation never rebuilds: it edits the annualised parameters, as it always has, so every change it makes is a From fb5ee57703141e6246c9f4cc77da851255f24265 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 12 Oct 2026 05:24:17 -0400 Subject: [PATCH 22/27] Test the batch rollback on a batch that fails after an update An unknown path is refused when targets are looked up, before anything changes, so the existing test never reached the rollback. A Reform.from_dict key reaches the overrides unparsed and fails after the first override has updated the rate; the new test checks the rate's values_list, its annual value, the recorded overrides and the modified flags on its path are all as before. It fails at 7819755de (flags left set) and passes here. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_dated_reform_routes.py | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/policyengine_uk/tests/test_dated_reform_routes.py b/policyengine_uk/tests/test_dated_reform_routes.py index d2a2091cb5..3c24e0bd33 100644 --- a/policyengine_uk/tests/test_dated_reform_routes.py +++ b/policyengine_uk/tests/test_dated_reform_routes.py @@ -1044,24 +1044,15 @@ def test_a_failed_override_batch_applies_none_of_its_overrides(): simulation = Simulation(situation=SITUATION) system = simulation.tax_benefit_system parameters = system.parameters - # Processing marks parameters modified, so clear the flags on the path - # to the rate to see whether a failed batch leaves them set. - path = [] - node = parameters.get_child(BASIC_RATE) - while node is not None: - node.modified = False - path.append(node) - node = getattr(node, "parent", None) with pytest.raises(ValueError): Scenario.from_reform( {BASIC_RATE: {"2027": 0.3}, "gov.not_a_real_parameter": {"2027": 1}} ).apply(simulation) assert annual(simulation, BASIC_RATE, 2027) == 0.20 assert system.legacy_overrides == [] - # The batch is undone in place: the tree is the one the system had, and - # nothing on the path to the rate is left marked modified. + # An unknown path is refused before anything changes, and the tree is + # the one the system had. assert system.parameters is parameters - assert not any(node.modified for node in path) Scenario.from_reform({BASIC_RATE: {"2027": 0.3}}).apply(simulation) assert system.parameters is parameters assert annual(simulation, BASIC_RATE, 2027) == 0.3 @@ -1070,6 +1061,32 @@ def test_a_failed_override_batch_applies_none_of_its_overrides(): assert annual(simulation, UC_SINGLE_OLD, 2027) == 500 +def test_a_batch_failing_after_an_update_restores_values_and_flags(): + simulation = Simulation(situation=SITUATION) + system = simulation.tax_benefit_system + rate = system.parameters.get_child(BASIC_RATE) + values_list = rate.values_list + # Processing marks parameters modified, so clear the flags on the path + # to the rate to see whether a failed batch leaves them set. + path, node = [], rate + while node is not None: + node.modified = False + path.append(node) + node = getattr(node, "parent", None) + # Reform.from_dict keys reach the overrides unparsed, so the second + # override fails after the first has updated the rate. + reform = Reform.from_dict( + {BASIC_RATE: {"2027-01-01": 0.3}, UC_SINGLE_OLD: {"not-a-date": 1}}, + country_id="uk", + ) + with pytest.raises(ValueError): + Scenario.from_reform(reform).apply(simulation) + assert rate.values_list is values_list + assert annual(simulation, BASIC_RATE, 2027) == 0.20 + assert system.legacy_overrides == [] + assert not any(node.modified for node in path) + + def test_a_rebuild_keeps_a_structural_reforms_parameter(): simulation = Simulation( situation=SITUATION, reform=(_add_bonus, {"test_budget.bonus": 7}) From 3bf52c5c37479ebf91062c75f78e4916ad420d3c Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 12 Oct 2026 05:28:44 -0400 Subject: [PATCH 23/27] Re-freeze the zero-gift test against main after #2352 The frozen expectations predated #2352 (savings and dividends above property income) and main's Scottish personal savings allowance on the UK bands, so the mixed-income rows and one allowance changed on main. Regenerated from main 3381500f4. With no gift, the branch matches main byte for byte on all 21 variables and the tax band. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/test_gift_aid.py | 67 ++++++++++---------------- 1 file changed, 25 insertions(+), 42 deletions(-) diff --git a/policyengine_uk/tests/test_gift_aid.py b/policyengine_uk/tests/test_gift_aid.py index 248b3a5b17..27d62c9d4c 100644 --- a/policyengine_uk/tests/test_gift_aid.py +++ b/policyengine_uk/tests/test_gift_aid.py @@ -270,6 +270,10 @@ def test_explicit_zero_gift_matches_omitted_input(sim, omitted_sim, drawn): def test_zero_gift_preserves_main_outputs_bit_for_bit(): """Freeze main's intermediate and final outputs, including mixed income. + Frozen from main at 3381500f4 (after #2352 put savings and dividends + above property income, and the Scottish personal savings allowance + moved to the UK bands). Re-freeze from main when main moves these. + Gift Aid's zero-input compatibility also applies to inherited baseline choices about property stacking, the Scottish schedule and taxpayer bands. Exact float32 expectations catch changes hidden by a tax-level tolerance. @@ -289,42 +293,28 @@ def test_zero_gift_preserves_main_outputs_bit_for_bit(): ) inputs["people"][f"p{i}"]["capital_gains"] = {YEAR: 40_000} snapshot = Simulation(situation=inputs) - allowance = [12_570, 12_570, 0, 12_570, 12_570, 12_070] * 2 expected = { "adjusted_net_income": [30_000, 60_000, 150_000, 69_000, 40_000, 101_000] * 2, - "personal_allowance": allowance, - "allowances": allowance, - "unused_personal_allowance": [0] * 12, + "personal_allowance": [12_570, 12_570, 0, 12_570, 12_570, 12_070] * 2, + "allowances": [12_570, 12_570, 0, 12_570, 12_570, 12_070] * 2, + "unused_personal_allowance": [0, 0, 0, 0, 0, 0] * 2, "earned_taxable_income": [17_430, 47_430, 150_000, 17_430, 0, 88_930] * 2, "received_allowances_earned_income": [12_570, 12_570, 0, 12_570, 10_000, 12_070] * 2, - "received_allowances_savings_income": [0, 0, 0, 0, 2_570, 0] * 2, - "received_allowances_dividend_income": [0] * 12, + "received_allowances_savings_income": [0, 0, 0, 0, 2570, 0] * 2, + "received_allowances_dividend_income": [0, 0, 0, 0, 0, 0] * 2, "basic_rate_earned_income": [17_430, 37_700, 37_700, 17_430, 0, 37_700] * 2, - "higher_rate_earned_income": [0, 9_730, 87_440, 0, 0, 51_230] * 2, + "higher_rate_earned_income": [0, 9730, 87_440, 0, 0, 51_230] * 2, "add_rate_earned_income": [0, 0, 24_860, 0, 0, 0] * 2, - "basic_rate_savings_income": [0, 0, 0, 6_500, 11_430, 0] * 2, - "higher_rate_savings_income": [0] * 12, - "add_rate_savings_income": [0] * 12, - "savings_allowance": [ - 1_000, - 500, - 0, - 500, - 1_000, - 500, - 1_000, - 500, - 0, - 500, - 1_000, - 0, - ], + "basic_rate_savings_income": [0, 0, 0, 10_770, 11_430, 0] * 2, + "higher_rate_savings_income": [0, 0, 0, 730, 0, 0] * 2, + "add_rate_savings_income": [0, 0, 0, 0, 0, 0] * 2, + "savings_allowance": [1000, 500, 0, 500, 1000, 500] * 2, "earned_income_tax": [ - 3_486, + 3486, 11_432, 53_703, - 3_486, + 3486, 0, 28_032, 3451.070068359375, @@ -334,31 +324,24 @@ def test_zero_gift_preserves_main_outputs_bit_for_bit(): 0, 31407.05078125, ], - "savings_income_tax": [0, 0, 0, 1_300, 2_286, 0] * 2, - "dividend_income_tax": [0, 0, 0, 4_313.75, 1_021.25, 0] * 2, - "property_income_tax": [0, 0, 0, 1_800, 0, 0, 0, 0, 0, 1_890, 0, 0], + "savings_income_tax": [0, 0, 0, 2446, 2286, 0] * 2, + "dividend_income_tax": [0, 0, 0, 6256.25, 1021.25, 0] * 2, + "property_income_tax": [0, 0, 0, 1800, 0, 0, 0, 0, 0, 1890, 0, 0], "income_tax": [ - 3_486, + 3486, 11_432, 53_703, - 10_899.75, - 3_307.25, + 13988.25, + 3307.25, 28_032, 3451.070068359375, 13182.0498046875, 60011.44921875, - 10954.8203125, - 3_307.25, + 14043.3203125, + 3307.25, 31407.05078125, ], - "capital_gains_tax": [ - 7663.7998046875, - 8_880, - 8_880, - 8_880, - 8263.7998046875, - 8_880, - ] + "capital_gains_tax": [7663.7998046875, 8880, 8880, 8880, 8263.7998046875, 8880] * 2, } for variable, values in expected.items(): From d594634979834ea8282f72f8212639f75523cfe9 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 12 Oct 2026 05:30:05 -0400 Subject: [PATCH 24/27] Make the rollback test fail if the batch stops failing after an update A spy on apply_legacy_overrides records that the batch raised inside the overrides after the rate was updated, so the test cannot pass without running the rollback if Reform.from_dict keys are ever checked earlier. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_dated_reform_routes.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/policyengine_uk/tests/test_dated_reform_routes.py b/policyengine_uk/tests/test_dated_reform_routes.py index 3c24e0bd33..da92893998 100644 --- a/policyengine_uk/tests/test_dated_reform_routes.py +++ b/policyengine_uk/tests/test_dated_reform_routes.py @@ -1061,7 +1061,7 @@ def test_a_failed_override_batch_applies_none_of_its_overrides(): assert annual(simulation, UC_SINGLE_OLD, 2027) == 500 -def test_a_batch_failing_after_an_update_restores_values_and_flags(): +def test_a_batch_failing_after_an_update_restores_values_and_flags(monkeypatch): simulation = Simulation(situation=SITUATION) system = simulation.tax_benefit_system rate = system.parameters.get_child(BASIC_RATE) @@ -1079,8 +1079,22 @@ def test_a_batch_failing_after_an_update_restores_values_and_flags(): {BASIC_RATE: {"2027-01-01": 0.3}, UC_SINGLE_OLD: {"not-a-date": 1}}, country_id="uk", ) + # Guard against the test going empty: the batch must fail inside the + # overrides, after the rate was updated, so the rollback runs. + original = tbs_module.apply_legacy_overrides + seen = {} + + def spy(parameters, overrides): + try: + return original(parameters, overrides) + except Exception: + seen["rate_updated"] = rate.values_list is not values_list + raise + + monkeypatch.setattr(tbs_module, "apply_legacy_overrides", spy) with pytest.raises(ValueError): Scenario.from_reform(reform).apply(simulation) + assert seen == {"rate_updated": True} assert rate.values_list is values_list assert annual(simulation, BASIC_RATE, 2027) == 0.20 assert system.legacy_overrides == [] From f5418da178071450166a684564654b5e936a75c3 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 12 Oct 2026 05:50:44 -0400 Subject: [PATCH 25/27] Describe the Gift Aid limits on the property-first band order Round-3 review nit. The design note listed two limitations the #2352 merge removed: savings and dividends now stack above property income, and the Personal Savings Allowance tests net income against the UK limits raised by the gift. It now describes that behaviour instead. Co-Authored-By: Claude Opus 5.5 --- docs/engineering/gift-aid.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/engineering/gift-aid.md b/docs/engineering/gift-aid.md index b64ef562c3..cdaf506147 100644 --- a/docs/engineering/gift-aid.md +++ b/docs/engineering/gift-aid.md @@ -37,21 +37,21 @@ adjusted net income. The Personal Allowance formula does not deduct Gift Aid a second time. Capital gains tax uses `net_income`, retains its gross Gift Aid band extension, and no longer removes the net gift from `allowances`. +Savings and dividends stack above all non-savings income, property income +included ([ITA 2007 s. 16](https://www.legislation.gov.uk/ukpga/2007/3/section/16); +#2352). Property income, savings and dividends therefore fill the bands in that +order, each against the extended limits. `non_savings_income_after_allowances`, +the base they stack on, starts from `net_income`, so the gift raises the limits +without also shrinking the income beneath them. The Personal Savings Allowance +compares net income less allowances with the UK basic and higher rate limits, +both extended by the gross gift ([s. 12B](https://www.legislation.gov.uk/ukpga/2007/3/section/12B), +including s. 12B(8) for Scottish taxpayers, with s. 414(2)(b)). + ## Existing limitations Zero-gift calculations retain their existing behavior. Several inherited limitations remain relevant when interpreting mixed-income results: -- Savings and dividend stacking currently omits property income from the - income beneath those slices. Gift Aid extends the limits of those existing - stacks. [ITA 2007 s. 16](https://www.legislation.gov.uk/ukpga/2007/3/section/16) - requires savings and dividends above other income; repairing property - occupancy separately would also change zero-gift results. -- The Personal Savings Allowance follows the model's `tax_band` enum, including - its Scottish band classification. [s. 12B(8)](https://www.legislation.gov.uk/ukpga/2007/3/section/12B) - instead tests Scottish non-savings income against the counterfactual UK - higher and additional rates. Extending the existing band tests preserves - that limitation. - The Scottish top-rate threshold parameter is £112,570 of income after allowances for 2024 onwards. The [HMRC rates and allowances table](https://www.gov.uk/government/publications/rates-and-allowances-income-tax/income-tax-rates-and-allowances-current-and-past) From 3ef3793bd7b2aaa675aad9c35146e5be59ab9018 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 12 Oct 2026 06:10:51 -0400 Subject: [PATCH 26/27] Name both stacking bases in the Gift Aid design note Round-4 review nit: property income is taxed on top of non_savings_income_after_allowances less property after allowances, and savings and dividends on top of non_savings_taxable_income. Both derive from net_income; they differ only with Marriage Allowance (#1947). Co-Authored-By: Claude Opus 5.5 --- docs/engineering/gift-aid.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/engineering/gift-aid.md b/docs/engineering/gift-aid.md index cdaf506147..20ac92abe7 100644 --- a/docs/engineering/gift-aid.md +++ b/docs/engineering/gift-aid.md @@ -40,9 +40,13 @@ band extension, and no longer removes the net gift from `allowances`. Savings and dividends stack above all non-savings income, property income included ([ITA 2007 s. 16](https://www.legislation.gov.uk/ukpga/2007/3/section/16); #2352). Property income, savings and dividends therefore fill the bands in that -order, each against the extended limits. `non_savings_income_after_allowances`, -the base they stack on, starts from `net_income`, so the gift raises the limits -without also shrinking the income beneath them. The Personal Savings Allowance +order, each against the extended limits. Property income is taxed on top of +`non_savings_income_after_allowances` less property income after allowances, +and savings and dividends on top of `non_savings_taxable_income` +(`earned_taxable_income` plus property income after allowances). Both bases +derive from `net_income`, so the gift raises the limits without also shrinking +the income beneath them. They differ only when Marriage Allowance applies, +which `earned_taxable_income` subtracts (#1947). The Personal Savings Allowance compares net income less allowances with the UK basic and higher rate limits, both extended by the gross gift ([s. 12B](https://www.legislation.gov.uk/ukpga/2007/3/section/12B), including s. 12B(8) for Scottish taxpayers, with s. 414(2)(b)). From 37cadb47192adb0110a796cddf5ef2cb2ce6d795 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 12 Oct 2026 06:53:46 -0400 Subject: [PATCH 27/27] Say which frozen objects the freeze keeps, and what unconfigure undoes From the GPT-6.1 Sol review of a8ff60fa0: frozen objects outside reference cycles are still freed by reference counting, and gc.unfreeze() also unfreezes anything frozen before the session. Comments only. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/tests/conftest.py | 15 ++++++++++----- .../tests/test_variable_module_release.py | 7 ++++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/policyengine_uk/tests/conftest.py b/policyengine_uk/tests/conftest.py index 1ec459c814..5f97662dbc 100644 --- a/policyengine_uk/tests/conftest.py +++ b/policyengine_uk/tests/conftest.py @@ -119,10 +119,13 @@ def pytest_collection_modifyitems(config, items): # - freezes every object that exists once the tests are collected # (gc.freeze(), about 1.6 million objects per worker): the modules, the # tax-benefit systems built at import and the collected tests. The collector -# never scans frozen objects, so a frozen object that later becomes garbage -# is never freed. No tax-benefit system may therefore die before the freeze -# (test_variable_module_release.py checks this); build one in a fixture or on -# first use, not at import. Objects created by tests are collected as before. +# never scans frozen objects, so frozen objects in reference cycles that +# later become garbage are not freed until they are unfrozen (objects outside +# cycles still go with their last reference). A tax-benefit system and its +# variable modules are such cycles, so no system may be built and dropped +# while the tests are collected (test_variable_module_release.py checks +# this); build one in a fixture or on first use, not at import. Objects +# created by tests are collected as before. # # Measured on CI on 11 October 2026 (runs and numbers in PR #2317): the # workers' CPU time, summed over the three shards, fell from 204 to 172 @@ -172,7 +175,9 @@ def pytest_collection_finish(session): def pytest_unconfigure(config): - # Leave a process that runs pytest in-process (an IDE, a REPL) as it was. + # Undo the freeze and threshold for a process that runs pytest in-process + # (an IDE, a REPL). gc.unfreeze() also unfreezes anything frozen before + # the session. global _heap_frozen if _heap_frozen: gc.unfreeze() diff --git a/policyengine_uk/tests/test_variable_module_release.py b/policyengine_uk/tests/test_variable_module_release.py index a0300c4538..241798a645 100644 --- a/policyengine_uk/tests/test_variable_module_release.py +++ b/policyengine_uk/tests/test_variable_module_release.py @@ -11,9 +11,10 @@ The conftest also freezes the objects that exist once the tests are collected (gc.freeze()). gc.get_objects() leaves frozen objects out, so the release must -still count a frozen system as live. A frozen object that later becomes -garbage is never freed, so no system may die before the freeze: its variable -modules would be frozen, then released and leaked. +still count a frozen system as live. Frozen objects in reference cycles that +later become garbage are not freed until they are unfrozen, so no system may be +built and dropped while the tests are collected: its variable modules would be +frozen, then released and leaked for the rest of the session. """ import gc