diff --git a/changelog.d/2775.fixed.md b/changelog.d/2775.fixed.md new file mode 100644 index 0000000000..2df91356de --- /dev/null +++ b/changelog.d/2775.fixed.md @@ -0,0 +1 @@ +The aggregate impact report's `small_target_change` condition now reads only the reform's own target, so a reform that moves another reform's target in fewer than 10 records keeps its costing and target change unless another condition withholds them. diff --git a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py index d3f83929d8..a8e54ba98d 100644 --- a/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py +++ b/policyengine_uk/tests/aggregate_impact/test_aggregate_impact_withheld_reasons.py @@ -6,7 +6,10 @@ tool withholds a year exactly when it records a reason, that the reasons match a direct computation, and that the years withheld and the figures published match the yes-or-no rule frozen at da31e8698 (the oracle in -``reform_withheld_main_reference.py``). +``reform_withheld_main_reference.py``) when its baseline holds the reform's +own target only. With other reforms' targets in the baseline, the frozen +rule checks them too and withholds more; since #2775 the tool checks the +reform's own target and no other. """ import re @@ -183,7 +186,8 @@ def _measure( ``base`` and ``reformed`` map variable to household values, or year to such a mapping. ``others`` are other reforms' targets, which measure() - checks too.""" + records in the baseline too; the target condition reads only the + reform's own (#2775).""" if not isinstance(next(iter(base)), int): base = {year: base for year in years} reformed = {year: reformed for year in years} @@ -297,10 +301,11 @@ def test_a_reform_that_changes_few_target_records_names_the_target(): ) -def test_a_small_change_in_another_reforms_target_names_that_target(): - """measure() checks every costed reform's target: the tool withholds a - reform aimed at income tax when it moves Pension Credit, another - reform's target, in three records.""" +def test_a_small_change_in_another_reforms_target_withholds_nothing(): + """#2775: a reform aimed at income tax moves Pension Credit, another + reform's target, in three records, and income tax in all 20. The report + publishes nothing about Pension Credit for this reform, so the tool + publishes its costing and target change.""" world = _world(np.full(N, 100.0), np.full(N, 100.0), pension_credit=np.zeros(N)) reformed = _world( np.full(N, 110.0), np.full(N, 100.0), pension_credit=_first(3, 25.0) @@ -319,10 +324,14 @@ def test_a_small_change_in_another_reforms_target_names_that_target(): baseline={YEAR: baseline_records(base_sim, _partition(), rec, YEAR, targets)}, components=_partition().component_variables(), ) - reasons = rec.metrics["reforms.raise_income_tax"]["withheld_because"] - assert _conditions(reasons[str(YEAR)]) == [ - (SMALL_TARGET_CHANGE, "pension_credit", None) - ] + for metric_id in ("reforms.raise_income_tax", "reform_targets.raise_income_tax"): + record = rec.metrics[metric_id] + assert record["withheld_because"] == {} and "suppressed" not in record + assert record["values"][str(YEAR)] == pytest.approx(200.0) + assert rec.checks["responsive:raise_income_tax"]["failing"][str(YEAR)] == ( + "expected income_tax up by at least £1,000,000,000,000 a year; " + "2026-27: income_tax moved only £200" + ) def test_a_year_without_baseline_records_says_so(): @@ -409,9 +418,10 @@ def test_component_metric_ids_name_heads_and_double_counts(): } -def test_two_small_targets_are_both_named(): - """The reform moves Pension Credit and Child Benefit, two other reforms' - targets, in three records each; every income tax bill rises too.""" +def test_of_two_small_targets_only_the_reforms_own_is_named(): + """The reform aimed at Pension Credit moves it and Child Benefit, another + reform's target, in three records each; every income tax bill rises too. + The tool names Pension Credit only.""" zero = np.zeros(N) base = _world( np.full(N, 100.0), np.full(N, 100.0), pension_credit=zero, child_benefit=zero @@ -422,13 +432,127 @@ def test_two_small_targets_are_both_named(): pension_credit=_first(3, 25.0), child_benefit=_first(3, 10.0), ) - rec = _measure(base, reformed, others=("pension_credit", "child_benefit")) + rec = _measure(base, reformed, target="pension_credit", others=("child_benefit",)) + assert _conditions(_reasons(rec)) == [(SMALL_TARGET_CHANGE, "pension_credit", None)] + assert rec.metrics["reform_targets.change_it"]["values"][str(YEAR)] is None + + +def test_a_small_balance_change_withholds_whatever_other_targets_do(): + """Three households' income tax rises, and so does Pension Credit, + another reform's target outside gov_balance: the tool still withholds the + reform aimed at the state pension for its three balances, and names only + them.""" + base = _world(np.full(N, 100.0), np.full(N, 100.0), pension_credit=np.zeros(N)) + reformed = _world( + _first(3, 110.0, 100.0), np.full(N, 100.0), pension_credit=_first(3, 5.0) + ) + rec = _measure(base, reformed, target="state_pension", others=("pension_credit",)) + assert _conditions(_reasons(rec)) == [(SMALL_BALANCE_CHANGE, "gov_balance", None)] + assert rec.metrics["reforms.change_it"]["values"][str(YEAR)] is None + + +def test_a_small_target_change_withholds_a_year_after_the_first(): + """In 2027 one household's income tax and state pension each rise by £1, + so no balance moves. The tool withholds that year for the reform's own + target alone.""" + base = _world(np.full(N, 10.0), np.full(N, 10.0)) + reformed = _world(_first(1, 11.0, 10.0), _first(1, 11.0, 10.0)) + rec = _measure(base, reformed, years=(2027,)) + assert _conditions(_reasons(rec, 2027)) == [ + (SMALL_TARGET_CHANGE, "income_tax", None) + ] + assert rec.metrics["reforms.change_it"]["values"]["2027"] is None + assert rec.metrics["reform_targets.change_it"]["values"]["2027"] is None + + +def test_a_reform_aimed_at_gov_balance_gives_both_reasons(): + """One household's balance moves, and gov_balance is the reform's own + target: the tool names it as the balance and as the target.""" + base = _world(np.full(N, 10.0), np.full(N, 10.0)) + reformed = _world(_first(1, 11.0, 10.0), np.full(N, 10.0)) + rec = _measure(base, reformed, target="gov_balance") assert _conditions(_reasons(rec)) == [ - (SMALL_TARGET_CHANGE, "pension_credit", None), - (SMALL_TARGET_CHANGE, "child_benefit", None), + (SMALL_BALANCE_CHANGE, "gov_balance", None), + (SMALL_TARGET_CHANGE, "gov_balance", None), ] +def test_a_reform_without_a_target_checks_no_other_reforms_target(): + """A reform that names no target raises every income tax bill and moves + Pension Credit, another reform's target, in three records: the tool + publishes its costing.""" + world = _world(np.full(N, 100.0), np.full(N, 100.0), pension_credit=np.zeros(N)) + reformed = _world( + np.full(N, 110.0), np.full(N, 100.0), pension_credit=_first(3, 25.0) + ) + base_sim = WorldSim({YEAR: world}, np.ones(N)) + rec = Recorder() + measure_reforms( + lambda reform=None: WorldSim({YEAR: reformed if reform else world}, np.ones(N)), + None, + {YEAR: weighted_total(base_sim, "gov_balance", YEAR)}, + {YEAR: {"pension_credit": 0.0}}, + parse_reforms({"reforms": [{"name": "Any", "parameters": {"a": 1}}]}), + [YEAR], + rec, + baseline={ + YEAR: baseline_records( + base_sim, _partition(), rec, YEAR, ["pension_credit"] + ) + }, + components=_partition().component_variables(), + ) + record = rec.metrics["reforms.any"] + assert record["withheld_because"] == {} and "suppressed" not in record + assert record["values"][str(YEAR)] == pytest.approx(200.0) + assert "reform_targets.any" not in rec.metrics + + +def test_a_baseline_without_the_reforms_own_target_fails_the_reform(): + """measure() records every reform's target in the baseline. A baseline + that lacks the reform's own fails the reform, rather than publishing a + year whose target the tool never checked.""" + world = _world(np.full(N, 100.0), np.full(N, 100.0)) + base_sim = WorldSim({YEAR: world}, np.ones(N)) + rec = Recorder() + outcomes = measure_reforms( + lambda reform=None: WorldSim( + {YEAR: _world(np.full(N, 110.0), np.full(N, 100.0)) if reform else world}, + np.ones(N), + ), + None, + {YEAR: weighted_total(base_sim, "gov_balance", YEAR)}, + {YEAR: {"income_tax": weighted_total(base_sim, "income_tax", YEAR)}}, + _reform("income_tax"), + [YEAR], + rec, + baseline={YEAR: baseline_records(base_sim, _partition(), rec, YEAR, [])}, + components=_partition().component_variables(), + ) + assert outcomes[0]["error"] == "KeyError: 'income_tax'" + assert "reforms.change_it" not in rec.metrics + assert rec.checks["responsive:change_it"]["failing"] == { + str(YEAR): "2026-27: the reform did not run" + } + + +def test_another_reforms_target_counts_in_the_costing_and_withholds_nothing(): + """The reform aimed at income tax raises it in all 20 households and the + state pension, another reform's target and a part of gov_balance, in + three. The tool publishes the costing, which holds those three pensions' + change, and the target change; the two together give that change, the + path privacy.py states (#2775).""" + base = _world(np.full(N, 100.0), np.full(N, 100.0)) + reformed = _world(np.full(N, 110.0), _first(3, 125.0, 100.0)) + rec = _measure(base, reformed, others=("state_pension",)) + assert _reasons(rec) == [] + costing = rec.metrics["reforms.change_it"]["values"][str(YEAR)] + target = rec.metrics["reform_targets.change_it"]["values"][str(YEAR)] + assert costing == pytest.approx(20 * 10.0 - 3 * 25.0) + assert target == pytest.approx(20 * 10.0) + assert target - costing == pytest.approx(3 * 25.0) + + def test_a_change_of_exactly_half_a_penny_changes_no_record(): """Three households hold nothing until the reform pays each of them a state pension of exactly CHANGE_GBP, which moves neither their balance @@ -759,12 +883,14 @@ def small(count): @settings(max_examples=300, deadline=None) @given(world=worlds()) def test_a_year_is_withheld_exactly_when_it_has_a_reason(world): - """For every world: the tool withholds the figures exactly when it - records a reason; the reasons match the conditions computed directly; - the years withheld match the frozen rule; and the published - figures equal the direct weighted changes.""" + """For every world, with the other variables as other reforms' targets: + the tool withholds the figures exactly when it records a reason; the + reasons match the conditions computed directly; the years withheld match + the frozen rule given the reform's own target; and the published figures + equal the direct weighted changes.""" base, reformed, weight, target = world - rec = _measure(base, reformed, weight, target) + others = tuple(v for v in HEADS + ("pension_credit",) if v != target) + rec = _measure(base, reformed, weight, target, others=others) reasons = _reasons(rec) for metric_id in ("reforms.change_it", "reform_targets.change_it"): record = rec.metrics[metric_id] @@ -784,8 +910,18 @@ def test_a_year_is_withheld_exactly_when_it_has_a_reason(world): components = partition.component_variables() frozen = main_reference.reform_withheld(reformed_sim, YEAR, records, components) assert frozen == bool(reasons) - assert reform_withheld(reformed_sim, YEAR, records, components) == frozen - assert withheld_reasons(reformed_sim, YEAR, records, components) == reasons + assert reform_withheld(reformed_sim, YEAR, records, components, target=target) == ( + frozen + ) + assert ( + withheld_reasons(reformed_sim, YEAR, records, components, target=target) + == reasons + ) + every = baseline_records(base_sim, partition, rec, YEAR, [target, *others]) + assert ( + withheld_reasons(reformed_sim, YEAR, every, components, target=target) + == reasons + ) costing = float(np.sum(weight * (_balance(reformed) - _balance(base)))) change = float(np.sum(weight * (reformed[target] - base[target]))) @@ -798,6 +934,76 @@ def test_a_year_is_withheld_exactly_when_it_has_a_reason(world): assert target_change == pytest.approx(change, abs=1e-6) +TARGETS = HEADS + ("pension_credit", "child_benefit") + + +@st.composite +def moves(draw): + """Five variables, each one reform's target; the reform moves each in + any records by one of a few amounts. Every value stays non-zero and 10 or + more households carry weight, so every component keeps 10 or more records + on both sides: only the balance and target conditions can hold.""" + weight = np.asarray( + draw(st.lists(st.sampled_from([1.0, 2.5]), min_size=N, max_size=N)) + ) + unweighted = draw(st.sets(st.integers(0, N - 1), max_size=N - MIN_CELL)) + weight[sorted(unweighted)] = 0.0 + base, reformed = {}, {} + for variable in TARGETS: + before = np.asarray(draw(st.lists(values, min_size=N, max_size=N))) + moved = np.isin(np.arange(N), sorted(draw(st.sets(st.integers(0, N - 1))))) + delta = draw(st.sampled_from([0.004, CHANGE_GBP, 0.006, 1.0, -0.5, 250.0])) + base[variable] = before + reformed[variable] = np.where(moved, before + delta, before) + return base, reformed, weight, draw(st.sampled_from(TARGETS)) + + +@settings(max_examples=300, deadline=None) +@given(world=moves()) +def test_a_year_is_withheld_exactly_when_its_own_target_or_balance_moves_in_few_records( + world, +): + """#2775, for every such world: the tool withholds a reform's year + exactly when its own target or gov_balance moves by more than CHANGE_GBP + in 1 to 9 records that carry weight, whichever other reforms' targets + move, and where. Measuring the reform with no other target gives the + same figures and checks. The frozen rule, given every target, withholds + exactly when this rule does or another reform's target moves by more + than CHANGE_GBP in 1 to 9 such records.""" + base, reformed, weight, target = world + others = tuple(v for v in TARGETS if v != target) + rec = _measure(base, reformed, weight, target, others=others) + + def few(before, after): + moved = (np.abs(after - before) > CHANGE_GBP) & (weight > 0) + return 0 < int(np.count_nonzero(moved)) < MIN_CELL + + expected = [] + if few(_balance(base), _balance(reformed)): + expected.append((SMALL_BALANCE_CHANGE, "gov_balance", None)) + if few(base[target], reformed[target]): + expected.append((SMALL_TARGET_CHANGE, target, None)) + assert _conditions(_reasons(rec)) == expected + for metric_id in ("reforms.change_it", "reform_targets.change_it"): + withheld = rec.metrics[metric_id]["values"][str(YEAR)] is None + assert withheld == bool(expected) + + alone = _measure(base, reformed, weight, target) + assert alone.metrics == rec.metrics and alone.checks == rec.checks + + partition = _partition() + every = baseline_records( + WorldSim({YEAR: base}, weight), partition, rec, YEAR, [target, *others] + ) + frozen = main_reference.reform_withheld( + WorldSim({YEAR: reformed}, weight), + YEAR, + every, + partition.component_variables(), + ) + assert frozen == (bool(expected) or any(few(base[v], reformed[v]) for v in others)) + + @settings(max_examples=300, deadline=None) @given(world=worlds()) def test_a_reason_never_gives_a_count_or_an_amount(world): diff --git a/policyengine_uk/utils/aggregate_impact/api.py b/policyengine_uk/utils/aggregate_impact/api.py index 26b5d7691a..89e72f465c 100644 --- a/policyengine_uk/utils/aggregate_impact/api.py +++ b/policyengine_uk/utils/aggregate_impact/api.py @@ -61,8 +61,8 @@ def to_report(result: dict) -> dict: carry ``withheld_because``: for a year the tool withholds, the conditions that withheld it, each as ``{"condition", "variable", "metric_id"}`` (``privacy.WITHHELD_CONDITIONS``); for a published year, an empty list. - A ``small_target_change`` names the variable, which can be another - reform's target: the tool checks the target of every reform it costs. + A ``small_target_change`` names the reform's own target, never another + reform's (#2775). A ``no_baseline`` reason stands alone: without a baseline record the tool tests no other condition, so the reason says nothing about the reform's records. diff --git a/policyengine_uk/utils/aggregate_impact/measure.py b/policyengine_uk/utils/aggregate_impact/measure.py index f2e32e5ae6..7679722349 100644 --- a/policyengine_uk/utils/aggregate_impact/measure.py +++ b/policyengine_uk/utils/aggregate_impact/measure.py @@ -1288,7 +1288,12 @@ def baseline_records(sim, partition: Partition, rec: "Recorder", year: int, targ def withheld_reasons( - reformed, year: int, baseline: Optional[Mapping], components + reformed, + year: int, + baseline: Optional[Mapping], + components, + *, + target: Optional[str], ) -> list: """Why the tool withholds a reform's costing and target change for ``year``: one reason per condition that holds, none when it publishes @@ -1307,10 +1312,12 @@ def withheld_reasons( whose change would give that new small cell back (``small_component_left``); - changes gov_balance in 1 to 9 households (``small_balance_change``), or - a target variable in 1 to 9 records (``small_target_change``), by more - than ``CHANGE_GBP``: the change is theirs alone, as for a decile row. - ``measure`` checks the target of every reform it costs, so the reason - names the variable: this reform's own target or another's. + its own target, ``target``, in 1 to 9 records (``small_target_change``), + by more than ``CHANGE_GBP``: the change is theirs alone, as for a + decile row. ``target`` is the variable the reform's ``responds`` names, + or None for a reform that names none. Another reform's target counts + here as it would if no reform aimed at it: in gov_balance, and in the + component conditions above when it is a component (#2775). Withheld too when there is no baseline record for the year (``no_baseline``). With nothing to compare against, the function tests @@ -1363,21 +1370,27 @@ def weight(variable): changed = np.abs(balance - baseline["balance"]) > CHANGE_GBP if small(_weighted_records(changed, baseline["household_weight"])): reasons.append(withheld_reason(SMALL_BALANCE_CHANGE, "gov_balance")) - for variable, before in baseline["targets"].items(): - after = array(reformed, variable, year).astype(np.float64) + if target is not None: + after = array(reformed, target, year).astype(np.float64) + before = baseline["targets"][target] if small( - _weighted_records(np.abs(after - before) > CHANGE_GBP, weight(variable)) + _weighted_records(np.abs(after - before) > CHANGE_GBP, weight(target)) ): - reasons.append(withheld_reason(SMALL_TARGET_CHANGE, variable)) + reasons.append(withheld_reason(SMALL_TARGET_CHANGE, target)) return reasons def reform_withheld( - reformed, year: int, baseline: Optional[Mapping], components + reformed, + year: int, + baseline: Optional[Mapping], + components, + *, + target: Optional[str], ) -> bool: """Whether the tool withholds a reform's costing and target change for ``year``: whether any condition of ``withheld_reasons`` holds.""" - return bool(withheld_reasons(reformed, year, baseline, components)) + return bool(withheld_reasons(reformed, year, baseline, components, target=target)) def _record_reasons(record: dict, year: int, reasons: list) -> None: @@ -1474,9 +1487,14 @@ def measure_reforms( parameters = reform_parameters(reform, baseline_parameters, years) outcome["parameters"] = parameters reformed = factory(reform=parameters) + target = None if reform.responds is None else reform.responds.variable for year in years: reasons = withheld_reasons( - reformed, year, (baseline or {}).get(year), components + reformed, + year, + (baseline or {}).get(year), + components, + target=target, ) if reasons: hidden_years[year] = reasons @@ -1493,16 +1511,15 @@ def measure_reforms( records=records, ) _record_reasons(rec.metrics[f"reforms.{reform.id}"], year, reasons) - if reform.responds is not None: - variable = reform.responds.variable + if target is not None: change = ( - weighted_total(reformed, variable, year) - - baseline_targets[year][variable] + weighted_total(reformed, target, year) + - baseline_targets[year][target] ) target_changes[year] = change rec.metric( f"reform_targets.{reform.id}", - f"{reform.name}: change in {variable}", + f"{reform.name}: change in {target}", "reform_targets", "gbp", year, diff --git a/policyengine_uk/utils/aggregate_impact/privacy.py b/policyengine_uk/utils/aggregate_impact/privacy.py index 85484f8ad6..d9bf2af6f7 100644 --- a/policyengine_uk/utils/aggregate_impact/privacy.py +++ b/policyengine_uk/utils/aggregate_impact/privacy.py @@ -42,11 +42,12 @@ component, so a reader can tell a reform whose own change is too small to publish from one that only touches another small cell. A published year records no reason, so it says nothing new. -- A withheld year's ``small_target_change`` that names the reform's own +- A withheld year's ``small_target_change``, which names the reform's own target, beside the ``responsive:`` verdict the report publishes for - that year, says whether fewer than 10 records together moved the target by - at least the reform's minimum: a bound on that cell's total, never its - value. A ``small_balance_change`` says much the same when the target is + that year, says whether the target moved by at least the reform's minimum + overall. Where changes of ``CHANGE_GBP`` or less add nothing, that bounds + the total of the fewer than 10 records it counts, never their value. A + ``small_balance_change`` says much the same when the target is part of gov_balance and nothing offsets it there. The counting rule above already publishes more (one household's £10,000 beside ten pennies). Reasons from several reform rows can together bound a cell, even to an @@ -54,6 +55,14 @@ records behind a published count of 10 show one record in each group), and the reform list, in CI the head's ``standard_reforms.yaml``, decides which rows exist. +- When gov_balance contains a reform's target, the reform's costing less + the target's part is the change in the rest of gov_balance, whose records + the tool doesn't count. A reform that moves its target in 10 or more + records, and the rest of gov_balance in a few households only, can + publish a costing and a target change that together give the rest's + change there. The tool checks the reform's own target only, so the two + figures can give the change in another reform's target too, as they can + for a variable no reform aims at. Threat model. The report defends against disclosure through any combination of the figures it publishes, across any number of reports, from honest