Stop calibrating OBR total NICs on ni_employee

changelog.d/obr-ni-total-target.fixed.md

@@ -0,0 +1 @@
+Stop calibrating OBR total NICs (Table 3.8, £200bn in 2025-26) on `ni_employee`, which the Class 1 employee NICs target (£50bn) also used; the two targets on one matrix column could not both be met. The Table 3.4 class targets (employee, employer, self-employed) already cover every NIC class the data populates. Tests now pin each OBR target's variable and check that no two calibration targets sharing a plain-variable column disagree on a year's value.

policyengine_uk_data/targets/build_loss_matrix.py

@@ -61,6 +61,22 @@
 logger = logging.getLogger(__name__)
 
 
+def calibration_targets() -> list[Target]:
+    """The national, regional and country targets the national matrix uses."""
+    targets = []
+    seen = set()
+    for level in (
+        GeographicLevel.NATIONAL,
+        GeographicLevel.REGION,
+        GeographicLevel.COUNTRY,
+    ):
+        for t in get_all_targets(geographic_level=level):
+            if t.name not in seen:
+                seen.add(t.name)
+                targets.append(t)
+    return targets
+
+
 def create_target_matrix(
     dataset,
     time_period: str = None,
@@ -89,23 +105,11 @@ def create_target_matrix(
 
     ctx = _SimContext(sim, time_period, dataset, reform)
 
-    all_targets = []
-    seen = set()
-    for level in (
-        GeographicLevel.NATIONAL,
-        GeographicLevel.REGION,
-        GeographicLevel.COUNTRY,
-    ):
-        for t in get_all_targets(geographic_level=level):
-            if t.name not in seen:
-                seen.add(t.name)
-                all_targets.append(t)
-
     df = pd.DataFrame()
     target_names = []
     target_values = []
 
-    for target in all_targets:
+    for target in calibration_targets():
         try:
             val = _resolve_value(target, year)
             if val is None:

policyengine_uk_data/targets/sources/obr.py

@@ -182,6 +182,7 @@ def _parse_receipts(wb: openpyxl.Workbook) -> list[Target]:
     the standard fiscal forecasting convention. Other receipts use the
     current-receipts table (cash basis) since they only appear there; that
     table is located by title because EFO vintages renumber the sheets.
+    NICs are not read here: ``_parse_nics`` targets them by class.
     """
     config = load_config()
     vintage = config["obr"]["vintage"]
@@ -237,9 +238,18 @@ def read_39(ws, row_num: int) -> dict[int, float]:
     # located by title rather than number: EFO vintages renumber the tables, and
     # in March 2026 current receipts moved to 3.8 while 3.9 became the APD
     # forecast, which silently yielded no targets at all.
+    #
+    # The "National insurance contributions" row is deliberately not targeted.
+    # It is total NICs, and it used to be targeted on ni_employee, which
+    # _parse_nics also targets at the Class 1 employee figure (£200bn against
+    # £50bn on one variable in 2025-26). Retargeting it on
+    # total_national_insurance would still conflict: the total also counts
+    # statutory payment recoveries and "Other NIC" (Class 1A, 1B and 3,
+    # settlements, unallocated). PE-UK has no variable for the first three, and
+    # no dataset fills Class 3 (#378). The class targets already cover every
+    # NIC class the data populates.
     ws39 = _find_receipts_sheet(wb)
     cash_rows = {
-        "ni": ("National insurance contributions", "ni_employee"),
         "vat": ("Value added tax", "vat"),
         "fuel_duties": ("Fuel duties", "fuel_duty"),
         "capital_gains_tax": ("Capital gains tax", "capital_gains_tax"),

policyengine_uk_data/tests/test_obr_receipts_sheet.py

@@ -122,12 +122,14 @@ def test_raises_when_rows_are_found_but_yield_no_values(monkeypatch):
 
 
 def test_parses_rows_when_values_are_present(monkeypatch):
-    """The same sheet with values yields one target per cash-basis row."""
+    """The same sheet with values yields one target per cash-basis row, except
+    total NICs, which the NICs parser targets by class instead."""
     monkeypatch.setattr(
         "policyengine_uk_data.targets.sources.obr.load_config",
         lambda: {"obr": {"vintage": "test", "efo_receipts": "https://example.invalid"}},
     )
     targets = _parse_receipts(_receipts_wb(populate=True))
     names = {target.name for target in targets}
     assert "obr/capital_gains_tax" in names
-    assert len(names) == 5
+    assert "obr/ni" not in names
+    assert len(names) == 4

policyengine_uk_data/tests/test_obr_target_conflicts.py

@@ -0,0 +1,156 @@
+"""Calibration targets that share a matrix column must agree.
+
+A target whose column is a plain sum (or recipient count) of one variable
+shares that column with every other such target on the same variable. Two of
+them with different values for one year contradict each other: no weights
+meet both, so the optimiser splits the difference and distorts everything
+else. obr/ni (total NICs, £200bn in 2025-26) and obr/ni_employee (£50bn) did
+exactly that on ni_employee.
+
+The column a target gets is decided by ``build_loss_matrix._compute_column``,
+so these tests ask that dispatcher rather than restating its rules: every
+custom compute function is swapped for a sentinel, so only targets that reach
+the plain-variable fallbacks get a column key.
+"""
+
+import math
+import random
+from unittest.mock import patch
+
+import pytest
+import requests
+
+from policyengine_uk_data.targets import build_loss_matrix as blm
+from policyengine_uk_data.targets.schema import Target, Unit
+from policyengine_uk_data.targets.sources import obr
+
+# Targets allowed to share a plain-variable column with different values: the
+# frozenset of their names, mapped to the reason. None are intended today.
+INTENDED_SHARED_COLUMNS: dict[frozenset[str], str] = {}
+
+
+@pytest.fixture
+def column_of(monkeypatch):
+    """Map a target to ("sum" | "count", variable, countries), or None."""
+    for name in dir(blm):
+        if name.startswith("compute_"):
+            monkeypatch.setattr(blm, name, lambda *args, **kwargs: None)
+    monkeypatch.setattr(blm, "_compute_simple_gbp", lambda t, ctx: ("sum", t.variable))
+    monkeypatch.setattr(
+        blm, "_compute_simple_count", lambda t, ctx: ("count", t.variable)
+    )
+
+    def column(target):
+        if target.custom_compute is not None:
+            return None
+        key = blm._compute_column(target, None, None)
+        if key is None:
+            return None
+        # Targets restricted to different countries get different columns
+        # (the ``countries`` field proposed in #490 and #530).
+        countries = getattr(target, "countries", None)
+        return key + (tuple(countries) if countries else None,)
+
+    return column
+
+
+def conflicts(targets, column) -> set[tuple]:
+    """(column, year, names) for every shared column whose values disagree."""
+    groups: dict[tuple, list[Target]] = {}
+    for target in targets:
+        if (key := column(target)) is not None:
+            groups.setdefault(key, []).append(target)
+    found = set()
+    for key, group in groups.items():
+        for year in {y for t in group for y in t.values}:
+            values = {
+                t.name: v
+                for t in group
+                if (v := blm._resolve_value(t, year)) is not None
+            }
+            first = next(iter(values.values()), None)
+            if any(not math.isclose(v, first, rel_tol=1e-9) for v in values.values()):
+                names = frozenset(values)
+                if names not in INTENDED_SHARED_COLUMNS:
+                    found.add((key, year, names))
+    return found
+
+
+@pytest.fixture(scope="module")
+def obr_targets():
+    obr._download_workbook.cache_clear()
+
+    def get(*args, **kwargs):
+        raise requests.ConnectionError("offline")
+
+    with (
+        patch.object(obr.requests, "get", side_effect=get),
+        patch.object(obr.time, "sleep", lambda s: None),
+    ):
+        targets = obr.get_targets()
+    obr._download_workbook.cache_clear()
+    return targets
+
+
+def test_obr_targets_agree_on_shared_columns(obr_targets, column_of):
+    assert conflicts(obr_targets, column_of) == set()
+
+
+def test_total_nics_on_ni_employee_is_flagged(obr_targets, column_of):
+    """The check fails on the mapping this PR removed."""
+    total_nics = Target(
+        name="obr/ni",
+        variable="ni_employee",
+        source="obr",
+        unit=Unit.GBP,
+        values={2025: 200.08e9},
+    )
+    found = conflicts(obr_targets + [total_nics], column_of)
+    assert {(key, names) for key, _, names in found} == {
+        (("sum", "ni_employee", None), frozenset({"obr/ni", "obr/ni_employee"}))
+    }
+    assert 2025 in {year for _, year, _ in found}
+
+
+def test_calibration_targets_agree_on_shared_columns(column_of):
+    """Every national, regional and country target the national matrix uses."""
+    assert conflicts(blm.calibration_targets(), column_of) == set()
+
+
+def _brute_force(targets, column) -> set[tuple]:
+    """Reference: compare every pair of targets directly."""
+    found = set()
+    for i, a in enumerate(targets):
+        for b in targets[i + 1 :]:
+            if column(a) is None or column(a) != column(b):
+                continue
+            for year in set(a.values) | set(b.values):
+                va, vb = blm._resolve_value(a, year), blm._resolve_value(b, year)
+                if None not in (va, vb) and not math.isclose(va, vb, rel_tol=1e-9):
+                    found.add((column(a), year))
+    return found
+
+
+def test_conflict_check_matches_pairwise_definition(column_of):
+    """Seeded random target sets: the grouped check flags exactly the
+    (column, year) pairs a pairwise comparison does. Covers custom columns,
+    counts against sums, and the nearest-earlier-year fallback."""
+    rng = random.Random(0)
+    for _ in range(300):
+        targets = []
+        for i in range(rng.randint(0, 8)):
+            is_count = rng.random() < 0.3
+            years = rng.sample(range(2020, 2027), rng.randint(1, 3))
+            targets.append(
+                Target(
+                    name=f"synthetic/{i}",
+                    variable=rng.choice(["a", "b", "c"]),
+                    source="synthetic",
+                    unit=Unit.COUNT if is_count else Unit.GBP,
+                    is_count=is_count,
+                    values={y: rng.choice([1.0, 2.0]) for y in years},
+                    custom_compute=(lambda *a: None) if rng.random() < 0.2 else None,
+                )
+            )
+        grouped = {(key, year) for key, year, _ in conflicts(targets, column_of)}
+        assert grouped == _brute_force(targets, column_of)

policyengine_uk_data/tests/test_obr_target_mapping.py

@@ -0,0 +1,103 @@
+"""Pin which PE-UK variable each OBR receipts target calibrates.
+
+The cash-receipts parser once targeted total NICs ("National insurance
+contributions", Table 3.8) on ``ni_employee``, the variable the NICs parser
+targets at Class 1 employee NICs alone. These tests read the committed EFO
+workbooks offline and pin the receipts targets' variables, the rows the NIC
+targets come from, and why total NICs has no target of its own.
+"""
+
+from unittest.mock import patch
+
+import openpyxl
+import pytest
+import requests
+
+from policyengine_uk_data.storage import STORAGE_FOLDER
+from policyengine_uk_data.targets.sources import obr
+
+# Every target parsed from the receipts workbook. Expenditure-side and static
+# OBR targets are left to their own tests.
+EXPECTED_VARIABLES = {
+    "obr/income_tax": "income_tax",
+    "obr/vat": "vat",
+    "obr/fuel_duties": "fuel_duty",
+    "obr/capital_gains_tax": "capital_gains_tax",
+    "obr/sdlt": "stamp_duty_land_tax",
+    "obr/ni_employee": "ni_employee",
+    "obr/ni_employer": "ni_employer",
+    "obr/ni_self_employed": "ni_self_employed",
+}
+
+# Table 3.4 rows under "National insurance contributions" in the committed
+# workbook: the class targets, then the rows the model leaves empty.
+CLASS_ROWS = {
+    "obr/ni_employee": "Class 1 Employee NICs",
+    "obr/ni_employer": "Class 1 Employer NICs",
+    "obr/ni_self_employed": "Class 4 and Class 2 Self employed NICs",
+}
+UNMODELLED_ROWS = ("Statutory payment recoveries", "Other NIC")
+
+
+@pytest.fixture(scope="module")
+def offline_targets():
+    obr._download_workbook.cache_clear()
+
+    def get(*args, **kwargs):
+        raise requests.ConnectionError("offline")
+
+    with (
+        patch.object(obr.requests, "get", side_effect=get),
+        patch.object(obr.time, "sleep", lambda s: None),
+    ):
+        targets = obr.get_targets()
+    obr._download_workbook.cache_clear()
+    return {t.name: t for t in targets}
+
+
+@pytest.fixture(scope="module")
+def receipts():
+    return openpyxl.load_workbook(
+        STORAGE_FOLDER / "obr_efo" / "efo_receipts.xlsx", data_only=True
+    )
+
+
+def _row_2025(ws, label: str, column: str) -> float:
+    """Value for 2025-26 (in £) of the row whose label starts with ``label``."""
+    return ws[f"{column}{obr._find_row(ws, label)}"].value * 1e9
+
+
+def test_receipts_target_variables_are_pinned():
+    wb = obr._fallback_workbook("efo-receipts")
+    targets = obr._parse_receipts(wb) + obr._parse_nics(wb)
+    assert {t.name: t.variable for t in targets} == EXPECTED_VARIABLES
+
+
+def test_total_nics_row_exists_but_has_no_target(offline_targets, receipts):
+    """The absence comes from the mapping, not from a missing row."""
+    cash = obr._find_receipts_sheet(receipts)
+    total = _row_2025(cash, "National insurance contributions", "E")
+    assert total > 150e9
+    assert "obr/ni" not in offline_targets
+    assert all(t.values.get(2025) != total for t in offline_targets.values())
+
+
+@pytest.mark.parametrize("name,label", CLASS_ROWS.items())
+def test_nic_class_targets_read_table_3_4(offline_targets, receipts, name, label):
+    expected = _row_2025(receipts["3.4"], label, "D")
+    assert offline_targets[name].values[2025] == pytest.approx(expected)
+
+
+def test_class_targets_and_unmodelled_rows_make_up_total_nics(receipts):
+    """Why total NICs is not targeted on total_national_insurance.
+
+    The class targets plus rows the model leaves empty (statutory payment
+    recoveries; Class 1A, 1B and 3 in "Other NIC") sum to the Table 3.4 total,
+    so a total target would demand NICs no household carries.
+    """
+    ws = receipts["3.4"]
+    total = _row_2025(ws, "National insurance contributions", "D")
+    classes = sum(_row_2025(ws, label, "D") for label in CLASS_ROWS.values())
+    unmodelled = sum(_row_2025(ws, label, "D") for label in UNMODELLED_ROWS)
+    assert unmodelled > 0
+    assert classes + unmodelled == pytest.approx(total, abs=0.02e9)