Repository navigation
Speed up the slowest property tests - #2819
Conversation
The NICs cap property builds one charge simulation for all its groups and both caps, and tests the fixed supplied-ANI populations once a year in their own test. The childcare hours property simulates its three variants in one simulation, and the income limit property reads the reform's own baseline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwQq6HkAr18afP7XnHdvKc
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwQq6HkAr18afP7XnHdvKc
…limit Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwQq6HkAr18afP7XnHdvKc
|
Ready for hub review Head: Items
Tests run
Mutation and revert checksEach ran against a scratch copy of
DifferentialsNone of these scripts is committed.
Judgement calls
CII checked once, just after opening the PR. Lint, Changelog fragments, Changes, Fix evidence and Aggregate impact were queued, and no Test shard had reported yet. Generated by Claude Code |
Aggregate impact on certified data: passBase Checks: 0 broken by this change, 1 failing on both sides, 0 fixed. Moves flagged for review: 0 (thresholds in Notes
Moves flagged for reviewNone. Spending by benefit and service
Caseloads
Standard reform costings (change in gov_balance)
The report suppresses a reform's costing and target change in these years, for the reasons below (
Standard reforms: change in each reform's target
Household net income by decileNo household's net income changes in any year. No change in any year: receipts by tax head; government balance, each component counted once; model totals; household income totals; population; poverty rates; people in poverty; inequality; mean household net income in each side's own deciles. Checks failing on both base and head (1)
Counted-once checkEach of 92 components was raised by £1 per unit in 2026-27, and every head of its totals was read household by household: its own head must rise by exactly £1, no other tax head may move, no other benefit may rise, and the model's own total must move by what its heads moved. A head that breaks a rule is measured again twice: with every other head held at its baseline value, and with every other head held at its value after the first £1 while the component rises again. It fails where either still shows the response. 0 failed; 0 were skipped.
Provenance
Dataset: Receipts by tax head: head levels
Spending by benefit and service: head levels
Caseloads: head levels
Government balance, each component counted once: head levels
Model totals: head levels
Household income totals: head levels
Population: head levels
Poverty rates: head levels
People in poverty: head levels
Inequality: head levels
Mean household net income in each side's own deciles: head levels
Standard reform costings (change in gov_balance): head levels
Standard reforms: change in each reform's target: head levels
|
|
Ready for hub review Verdict: APPROVE WITH NITS at Neither file loses a check. CI confirms the speed-up: the statutory file drops from 2,083 to 1,022 node seconds (−51%) and the childcare file from 824 to 476 (−42%). Every differential and mutation I reran agrees with the body. The findings below are text fixes, plus one follow-up test for an income-limit mutation that FindingsP1None. P2None. P3P3-1. The body understates the supplied-ANI change: the head checks the expected arrays exactly under both caps, and
The head runs these checks in
with:
P3-2. The body counts the surviving income-limit mutations as two; the hand-off counts three. (the body's "Mutation checks", last paragraph) The hand-off lists M3, M3b and a both-routes mutation; the body says "Two more". I confirmed one of them (M3, below). On the second route,
P3-3. The changelog fragment leaves out the income-limit property. ( The income-limit property in the childcare file also builds one simulation fewer, since it now reads
with:
P3-4. Two docstrings use the passive voice, and one describes the change rather than the code. ( Replace:
with:
and replace:
with:
P3-5. The body could report the CI timings, which now exist. (the body's "Timings") The body gives local medians only and calls them noisy. You could add a row or a sentence: "On CI ( P3-6 (follow-up, not caused by this PR). An explicit test kills the reversed income-limit comparison that both property versions miss. I confirmed the survivor in a scratch copy. I changed The four passes are the two @pytest.mark.parametrize("high, funded", [(100_000, True), (100_001, False)])
def test_income_limit_admits_exactly_the_couples_within_it(high, funded):
"""A working couple's 3-year-old gets working parent hours exactly when
both parents' adjusted net income is at most £100,000 (SI 2022/1134 regs
14(3)(c)(i) and 15(3)(b)(i))."""
year = 2026
people = {
"parent1": {
"age": {year: 35},
"employment_income": {year: high},
"is_parent": {year: True},
},
"parent2": {
"age": {year: 35},
"employment_income": {year: 20_000},
"is_parent": {year: True},
},
"child0": {"age": {year: 3}},
}
members = list(people)
situation = {
"people": people,
"benunits": {
"bu": {
"members": members,
"maximum_extended_childcare_hours_usage": {year: 30},
}
},
"households": {"hh": {"members": members, "country": {year: "ENGLAND"}}},
}
sim = new_simulation(situation=situation)
extended = float(sim.calculate("extended_childcare_entitlement", year).sum())
assert (extended > 0) == funded, (high, extended)The regulation numbers in the docstring come from the model's own Checked and sound
What I couldn't check
Generated by Claude Code |
|
Picking this up for the UK hub. Reply here if you're mid-change and I'll stop. Generated by Claude Code |
… in the changelog Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwQq6Jv2xFDo21pssDX3FN
…n it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwQq6Jv2xFDo21pssDX3FN
|
Generated by Claude Code |
|
Generated by Claude Code |
|
Ready for hub review Head:
I did not push to Before I started, I checked: the last push to Findings
Citation and figure check (P3-6)
Mutation (M4)I ran this in a scratch worktree of this head, with
CI timings (P3-5)I reran
This matches the review exactly: −51% and −42%. Tests at
|
juaristi22
left a comment
There was a problem hiding this comment.
Approving exact head 79da970fc25d632efa29d600e0e19424961cd4ed. Review COMPLETE, with no findings or material evidence gaps, and current required CI passes.
The refactors preserve the assertions, strategies, example counts and tolerances, with separate entities and the DC/DB paths retained. Seven focused checks passed, including the full hours property, both supplied-ANI years, the exact childcare boundaries and fresh-versus-reused baseline/input-isolation diagnostics. The omitted full-file local repeats are covered separately by successful exact-head hosted shards/YAML/Test Result and the recorded static coverage checks.
The review establishes functional coverage and isolation; the author's before/after timing percentages and mutation campaign were not rerun or independently certified. This remains a tests-only change with an appropriate policy/Axiom n/a.
Addresses CI shard time
Reproducer: n/a: this changes tests only
axiom: n/a: tests only
This PR changes two of the slowest test files. It touches no formula, parameter, input, fixture that a production path reads, or CI workflow. Every strategy,
max_examplessetting and assertion stays.What the tests do now
test_statutory_pension_edges.pytest_nics_cap_preserves_aa_inputs_and_charge_with_fixed_ordinary_taxused to build 16 simulations for each drawn example. It now builds 5. The drawn DC and DB populations still get their own simulation for each cap. One charge simulation now holds every group under both caps. Each person in it has their own benefit unit and household and supplies the same inputs as everyone else, so no charge reads another person's._charges_with_fixed_ordinary_tax_inputsreturns each set's charges and checks each set againstexcess × 0.45.test_nics_cap_preserves_aa_inputs_and_charge_with_supplied_ani[2029|2030], with the same expected arrays. The test asserts equality for each array under both caps;mainasserted equality only with no cap, and checked the capped results to within 0.05. The DC witness[130_000, 304_600]gets the same check under both caps. This adds two node ids and renames or removes none, soshard_durations.tomlneeds no change.test_childcare_entitlement_floor_properties.pytest_more_hours_used_never_lowers_childcare_supportused to build three simulations for each example. It now builds one, holding the unbumped, cap-bumped and hours-bumped copies of the families.simulatetakes avariantslist. Each copy has its own benefit units and households and supplies the same inputs.@examplewith two 3-year-olds in England: one whose family's maximum hours used (40) exceed the working-parent total, and one whose own hours used (20) bind below the family maximum.test_raising_the_income_limit_never_lowers_support_or_incomenow reads the current-law side fromreform.baseline.Simulation.__init__builds that baseline asSimulation(scenario=None, situation=situation, ...), which matches the separate simulation the test used to build. That is one simulation fewer per example.What else this PR adds
test_income_limit_admits_exactly_the_couples_within_it[100000-True|100001-False]gives a working couple in England a 3-year-old, 30 hours used and a second parent on £20,000, and checks that the child gets extended hours when the first parent's adjusted net income comes to £100,000 and none when it comes to £100,001. SI 2022/1134 reg. 14(3)(c)(i) and reg. 15(3)(b)(i) require that the parent and the partner do not "expect their adjusted net income to exceed £100,000", andgov.dfe.extended_childcare_entitlement.income.limitsets £100,000 for 2026. The gap this test closes predates this PR: the property onmainalso misses the reversed comparison (see "Mutation checks").Timings
On CI (
.github/test_durations.pyover the shard logs), the statutory file takes 2,083 → 1,022 node seconds (−51%) and the childcare file 824 → 476 (−42%). The "after" figures come from the six Python shards ofc7f54e405; the "before" figures come from the seven Python shards of run 41579222743, a PR run onmainwhose branch leaves both files asmainhas them.Locally, the machine had 4 cores and ran
pytest <file> -n 4 --durations=0. Each file ran three times before and three times after. The figures are medians of node seconds (setup, call and teardown summed). The "before" runs usedorigin/mainat2da212abin a separate worktree. This machine runs slower than CI: the property node takes about 1,100 s here and 760 s on CI.test_statutory_pension_edges.pytest_childcare_entitlement_floor_properties.pyBy node (local medians, before → after):
test_nics_cap_preserves_aa_inputs_and_charge_with_fixed_ordinary_tax[2029]: 1109 → 380test_nics_cap_preserves_aa_inputs_and_charge_with_fixed_ordinary_tax[2030]: 1164 → 389test_nics_cap_preserves_aa_inputs_and_charge_with_supplied_ani[2029]/[2030](new): 45 / 43test_more_hours_used_never_lowers_childcare_support: 359 → 140test_raising_the_income_limit_never_lowers_support_or_income: 286 → 224test_becoming_eligible_never_lowers_childcare_support(unchanged): 143 → 157Mutation checks
Each check mutated a copy of
origin/main, then ran the old test and the new test against it. Shrinking was off (phases=[explicit, reuse, generate]) so failures report quickly. For M4, the old test file ismain's and the new one is this head's, each withtest_income_limit_admits_exactly_the_couples_within_itappended, run in a copy of this head with only the formula mutated.pension_annual_allowance_threshold_incomereadspension_contributions_via_salary_sacrifice_adjusted(capped) in place of..._from_payfixed_ordinary_taxnodes failfixed_ordinary_taxnodes and bothsupplied_aninodes failchildcare_term_valuelimits working hours by60 - family_hoursextended_childcare_entitlement_meets_income_requirements,extended_childcare_entitlement_work_condition) also requirep.income.limit < np.infextended_childcare_entitlement_meets_income_requirementstestsani > p.income.limitin place ofani <= p.income.limittest_income_limit_admits_exactly_the_couples_within_it[100000-True]failsThe income-limit property misses M4, and M4 with the limited-capability route reversed too, in the old and the new test alike, so neither miss is a regression. M4 makes both the baseline and the reform ineligible: the lower earner then fails the reversed test under either limit, and both parents must meet it. Reversing the limited-capability route as well changes nothing more in 2026, because the Department for Education stopped applying the limit on that route from 1 December 2022 (
exemption/income_limit_applies.yaml).Differentials
Two uncommitted scripts compared the old per-case path with the new one.
np.array_equal).reform.baselinegave identical arrays for the three entitlements andhousehold_net_income.Checks run
-n 4 --durations=0: 43 passed in 672.26 s (statutory) and 22 passed in 311.98 s (childcare, now 22 nodes with the two new ones). The new nodes' calls take 5.66 s ([100000-True]) and 6.07 s ([100001-False]).pytest policyengine_uk/tests/code_health -n 4: 3087 passed.ruff@0.17.2 format --check .,ruff check .andruff check --select F811,F821 policyengine_ukpass, as doesgit diff --check.Not changed
Open PRs change these files from the brief, so this PR leaves them alone:
test_tax_credits_pension_credit_passport_properties.py,test_uc_single_claim_properties.pyandtest_pension_credit_child_amount_tax_credit_properties.py(#2077), andtest_council_tax_reduction_young_person_properties.py(#2301, #2302). No PR has started the rest of the list yet.🤖 Generated with Claude Code