Drop the clone override now that policyengine-core keeps each clone's input record

changelog.d/2260.changed.md

@@ -0,0 +1 @@
+- Require policyengine-core 3.32.27, which gives each clone its own record of `set_input` values and forgets the record of a value `Holder.delete_arrays` deletes (PolicyEngine/policyengine-core#561), and remove the `Simulation.clone` override that worked around policyengine-core#559. The `Simulation.delete_arrays` override and the `Simulation.calculate` sweep stay, now for the records that rebuilding the populations leaves behind (PolicyEngine/policyengine-core#605).

policyengine_uk/simulation.py

@@ -235,26 +235,13 @@ def __init__(
             if scenario.parameter_changes is not None:
                 self.apply_parameter_changes(scenario.parameter_changes)
 
-    def clone(
-        self,
-        debug: bool = False,
-        trace: bool = False,
-        clone_tax_benefit_system: bool = True,
-    ) -> "Simulation":
-        clone = super().clone(debug, trace, clone_tax_benefit_system)
-        # policyengine-core 3.32.9: simulations/simulation.py::Simulation.clone
-        # shallow-copies __dict__, while holders/holder.py::Holder.clone copies
-        # value storage. Holder.set_input's provenance and context must belong
-        # to the same simulation as that storage, including for plain clones.
-        clone._user_input_keys = set(getattr(self, "_user_input_keys", ()))
-        clone._user_input_contexts = list(getattr(self, "_user_input_contexts", ()))
-        return clone
-
     def delete_arrays(self, variable: str, period: Period = None) -> None:
         super().delete_arrays(variable, period)
-        # policyengine-core's Simulation.delete_arrays and
-        # holders/holder.py::Holder.delete_arrays remove storage, but retain
-        # provenance keys. Drop those keys before carry-over can refill storage.
+        # From policyengine-core 3.32.27 (policyengine-core#561) the deletion
+        # forgets the input records of the values it removes, but not records
+        # that a population rebuild left with no stored value
+        # (policyengine-core#605). Drop those too, before carry-over can refill
+        # the period and input export would count the refill as an input.
         # Inspect storage after core's deletion to honour period containment
         # and the current branch, ancestor branches and default branch.
         drop_missing_supplied_inputs(self, variable)
@@ -659,9 +646,11 @@ def calculate(
         period = period_(period)
 
         if variable_name in SUPPLIED_INPUT_VARIABLES:
-            # A value deleted straight from the holder leaves its input record
-            # behind. Forget it before the engine can refill the period with a
-            # carried-over value that the record would pass off as supplied.
+            # Rebuilding the populations leaves the input records of the
+            # replaced holders behind (policyengine-core#605). Forget a record
+            # whose stored value is gone before the engine can refill the
+            # period with a carried-over value that the record would pass off
+            # as supplied.
             drop_missing_supplied_inputs(self, variable_name)
 
         return super().calculate(

policyengine_uk/tests/test_input_records.py

@@ -0,0 +1,32 @@
+"""A simulation exports as inputs only the values supplied to it.
+
+policyengine-core records each (variable, branch, period) that ``set_input``
+stores, and ``to_input_dict`` exports the recorded values. Rebuilding the
+populations keeps the record of inputs the rebuild drops
+(policyengine-core#605). ``Simulation.delete_arrays`` forgets such a record,
+so a value the engine later carries into that period is not exported as an
+input.
+"""
+
+import pytest
+
+from policyengine_uk import Simulation
+
+pytestmark = pytest.mark.usefixtures("cloned_uk_tax_benefit_system")
+
+
+def situation(hours):
+    return {
+        "people": {"person": {"age": {2025: 40}, "hours_worked": hours}},
+        "benunits": {"benunit": {"members": ["person"]}},
+        "households": {"household": {"members": ["person"]}},
+    }
+
+
+def test_deleting_after_a_rebuild_does_not_export_a_carried_over_input():
+    sim = Simulation(situation=situation({2025: 1_000, 2026: 500}))
+    sim.build_from_situation(situation({2025: 1_000}))
+    sim.delete_arrays("hours_worked", 2026)
+    # The engine carries 2025's hours into 2026; they are not an input.
+    assert sim.calculate("hours_worked", 2026).tolist() == [1_000]
+    assert sim.to_input_dict()["hours_worked"] == {"2025": [1_000]}

policyengine_uk/tests/test_ni_class_4_profits_properties.py

@@ -45,7 +45,7 @@
 
 import numpy as np
 import pytest
-from hypothesis import HealthCheck, given, settings
+from hypothesis import HealthCheck, example, given, settings
 from hypothesis import strategies as st
 from policyengine_core.periods import period as period_
 from policyengine_core.reforms import Reform
@@ -754,9 +754,12 @@ def test_class_4_does_not_count_a_deleted_loss_after_recalculation(on_branch):
 def test_supplied_input_helpers_ignore_missing_stored_arrays():
     sim = single_person({"trading_loss": {2025: 10_000, 2026: 5_000}})
     population = sim.get_variable_population("trading_loss")
-    # Direct holder deletion leaves core's provenance keys behind. Neither
-    # helper may identify the missing stored value as an effective input.
     population.get_holder("trading_loss").delete_arrays(period_(2026))
+    # Core forgets the deleted input's record (policyengine-core#561). Put it
+    # back, as a population rebuild would leave it (policyengine-core#605):
+    # neither helper may identify the missing stored value as an effective
+    # input.
+    sim._user_input_keys.add(("trading_loss", "default", period_(2026)))
     assert supplied_input(population, "trading_loss", period_(2026)) is None
     assert supplied_input_periods(population, "trading_loss") == [period_(2025)]
 
@@ -773,10 +776,11 @@ def test_class_4_does_not_count_a_loss_deleted_from_its_holder(on_branch, delete
     )
     sim = original.get_branch("deleted_loss") if on_branch else original
     sim.set_input("trading_loss", 2026, np.array([deleted_loss]))
-    # Holder.delete_arrays removes the stored value but not core's record of
-    # it. The engine then carries the 2025 loss into 2026; with the stale
-    # record that 10,000 would count as a second loss (profits of 20,000 and
-    # contributions of 445.80), also when the deleted loss was 10,000 too.
+    # The engine carries the 2025 loss into the emptied 2026. Before
+    # policyengine-core 3.32.27, Holder.delete_arrays kept core's record of
+    # the deleted input (policyengine-core#559), and that 10,000 would count
+    # as a second loss (profits of 20,000 and contributions of 445.80), also
+    # when the deleted loss was 10,000 too.
     sim.get_holder("trading_loss").delete_arrays(period_(2026), sim.branch_name)
     assert sim.calculate("trading_loss", 2026)[0] == 10_000
     population = sim.get_variable_population("trading_loss")
@@ -792,6 +796,147 @@ def test_class_4_does_not_count_a_loss_deleted_from_its_holder(on_branch, delete
         assert original.calculate("ni_class_4_profits", 2026)[0] == 30_000
 
 
+def test_class_4_does_not_count_a_loss_a_rebuild_drops():
+    def situation(losses):
+        person = {
+            "age": {2025: 40},
+            "self_employment_income": {2026: 40_000},
+            "trading_loss": losses,
+        }
+        return {
+            "people": {"person": person},
+            "benunits": {"benunit": {"members": ["person"]}},
+            "households": {"household": {"members": ["person"]}},
+        }
+
+    sim = Simulation(situation=situation({2025: 10_000, 2026: 5_000}))
+    # Rebuilding the populations replaces every holder but keeps core's
+    # record of the 2026 loss (policyengine-core#605). The engine carries the
+    # 2025 loss into 2026; with the stale record that 10,000 would count as a
+    # second loss (profits of 20,000 and contributions of 445.80).
+    sim.build_from_situation(situation({2025: 10_000}))
+    assert sim.calculate("trading_loss", 2026)[0] == 10_000
+    population = sim.get_variable_population("trading_loss")
+    assert supplied_input(population, "trading_loss", period_(2026)) is None
+    assert supplied_input_periods(population, "trading_loss") == [period_(2025)]
+    observed = (
+        float(sim.calculate("ni_class_4_profits", 2026)[0]),
+        round(float(sim.calculate("ni_class_4", 2026)[0]), 2),
+    )
+    assert observed == (30_000, 1_045.80)
+
+
+LIFECYCLE_YEARS = (2024, 2025, 2026)
+lifecycle_loss = st.integers(min_value=0, max_value=3).map(lambda k: 1_000.0 * k)
+LIFECYCLE_STEPS = 6
+# Each step names the simulation it acts on, modulo the number made so far
+# (at most one more than the steps), so an original keeps changing after it
+# has been cloned.
+lifecycle_step = st.tuples(
+    st.integers(min_value=0, max_value=LIFECYCLE_STEPS),
+    st.one_of(
+        st.tuples(st.just("set"), st.sampled_from(LIFECYCLE_YEARS), lifecycle_loss),
+        st.tuples(
+            st.sampled_from(
+                ["delete", "delete_from_holder", "calculate", "calculate_profits"]
+            ),
+            st.sampled_from(LIFECYCLE_YEARS),
+        ),
+        st.tuples(st.just("clone")),
+        st.tuples(st.just("rebuild"), st.frozensets(st.sampled_from(LIFECYCLE_YEARS))),
+    ),
+)
+
+
+def lifecycle_situation(losses):
+    person = {
+        "age": {2024: 40},
+        "self_employment_income": {year: 20_000 for year in LIFECYCLE_YEARS},
+        "trading_loss": losses,
+    }
+    return {
+        "people": {"person": person},
+        "benunits": {"benunit": {"members": ["person"]}},
+        "households": {"household": {"members": ["person"]}},
+    }
+
+
+_lifecycle_templates = []
+
+
+def lifecycle_simulation(losses):
+    # Rebuild a clone of one template, as the State Pension age properties do,
+    # so each example also starts from a population rebuild.
+    if not _lifecycle_templates:
+        _lifecycle_templates.append(
+            Simulation(situation=lifecycle_situation({2025: 2_000.0}))
+        )
+    sim = _lifecycle_templates[0].clone(clone_tax_benefit_system=False)
+    sim.build_from_situation(lifecycle_situation(losses))
+    return sim
+
+
+def assert_supplied_losses(sim, expected):
+    population = sim.get_variable_population("trading_loss")
+    for year in LIFECYCLE_YEARS:
+        value = supplied_input(population, "trading_loss", period_(year))
+        assert (None if value is None else value.tolist()) == (
+            [expected[year]] if year in expected else None
+        )
+    assert supplied_input_periods(population, "trading_loss") == [
+        period_(year) for year in sorted(expected)
+    ]
+
+
+@PROPERTY_SETTINGS
+@given(
+    st.dictionaries(st.sampled_from(LIFECYCLE_YEARS), lifecycle_loss),
+    st.lists(lifecycle_step, max_size=LIFECYCLE_STEPS),
+)
+# The rebuild leaves the template's 2025 record behind, and building the
+# dataset calculates 2025 (policyengine-core#605).
+@example(initial={2024: 0.0}, steps=[])
+# With one record between a simulation and its clone, the input set on one
+# passes off the other's calculated 2025 as supplied (policyengine-core#559).
+@example(initial={2024: 0.0}, steps=[(0, ("clone",)), (0, ("set", 2025, 0.0))])
+def test_only_losses_set_and_kept_count_as_supplied(initial, steps):
+    # A loss counts as supplied exactly while the last value set for it, by
+    # set_input or a rebuild, has not been deleted or dropped by a rebuild,
+    # whatever the engine has carried over or calculated meanwhile. A clone
+    # starts with its original's supplied losses, and what either does
+    # afterwards does not reach the other.
+    sim = lifecycle_simulation(initial)
+    simulations = [(sim, dict(initial))]
+    assert_supplied_losses(sim, initial)
+    for which, step in steps:
+        sim, expected = simulations[which % len(simulations)]
+        kind = step[0]
+        if kind == "set":
+            sim.set_input("trading_loss", step[1], np.array([step[2]]))
+            expected[step[1]] = step[2]
+        elif kind == "delete":
+            sim.delete_arrays("trading_loss", step[1])
+            expected.pop(step[1], None)
+        elif kind == "delete_from_holder":
+            sim.get_holder("trading_loss").delete_arrays(period_(step[1]))
+            expected.pop(step[1], None)
+        elif kind == "calculate":
+            sim.calculate("trading_loss", step[1])
+        elif kind == "calculate_profits":
+            sim.calculate("ni_class_4_profits", step[1])
+        elif kind == "clone":
+            simulations.append(
+                (sim.clone(clone_tax_benefit_system=False), dict(expected))
+            )
+        elif kind == "rebuild":
+            kept = {year: expected[year] for year in step[1] if year in expected}
+            sim.build_from_situation(lifecycle_situation(kept))
+            expected.clear()
+            expected.update(kept)
+        for simulation, supplied in simulations:
+            assert_supplied_losses(simulation, supplied)
+
+
 def test_supplied_input_helpers_refuse_an_unregistered_variable():
     sim = single_person({"employment_income": {2026: 20_000}})
     population = sim.get_variable_population("employment_income")

policyengine_uk/utils/supplied_inputs.py

@@ -4,17 +4,21 @@
 not set for a period may inherit a previously known value
 (``auto_carry_over_input_variables``), and calculated values are cached next to
 inputs. A holder's known periods include both. policyengine-core records each
-(variable, branch, period) that ``set_input`` fills in
-``Simulation._user_input_keys``. UK simulations isolate this provenance on
-cloning and remove keys when deleting arrays; the helpers also require a
-stored value, so missing arrays do not count as supplied inputs.
-
-``Holder.delete_arrays`` removes a stored value without removing its record
-(policyengine-core#559). The engine could then carry an earlier value into the
-empty period, and the stale record would pass it off as supplied. So
-``Simulation.calculate`` forgets the records of a variable in
+(variable, branch, period) that ``set_input`` stores in
+``Simulation._user_input_keys``. From 3.32.27 (policyengine-core#561), core
+forgets the record of an input that ``Holder.delete_arrays`` deletes, and each
+clone gets its own copy of the record. The helpers also require a stored
+value, so missing arrays do not count as supplied inputs.
+
+``build_from_situation`` installs fresh populations, and so fresh holders,
+through ``Simulation.build_from_populations``, which keeps the record
+(policyengine-core#605). The engine could then carry an earlier value into a
+period whose input the rebuild dropped, and the stale record would pass it off
+as supplied. So ``Simulation.calculate`` forgets the records of a variable in
 ``SUPPLIED_INPUT_VARIABLES`` whose stored value is gone before the engine can
 refill it, and the helpers only answer for those variables.
+``Simulation.delete_arrays`` forgets such records too, for any variable it
+deletes.
 """
 
 from typing import List, Optional
@@ -60,7 +64,7 @@ def _visible_branch_names(simulation) -> List[str]:
 
 def drop_missing_supplied_inputs(simulation, variable_name: str) -> None:
     """Forget each record of ``variable_name`` as an input, on this branch
-    or one it was made from, whose stored value has been deleted."""
+    or one it was made from, whose stored value is gone."""
     input_keys = getattr(simulation, "_user_input_keys", None)
     if not input_keys:
         return
@@ -112,7 +116,7 @@ def supplied_input(
         if (variable_name, branch_name, period) in input_keys:
             # Read the stored input itself: Holder.get_array would fall back
             # to other branches' values, including cached calculations.
-            # Holder.delete_arrays leaves provenance keys behind; a missing
+            # A record can outlive its stored array (see above); a missing
             # stored array must be ignored even if its key remains.
             value = holder._get_array_from_storage(period, branch_name)
             if value is not None:

pyproject.toml

@@ -24,7 +24,7 @@ classifiers = [
 ]
 requires-python = ">=3.11"
 dependencies = [
-    "policyengine-core>=3.32.12",
+    "policyengine-core>=3.32.27",
     "microdf-python>=1.2.1",
     "pydantic>=2.11.7",
     "tables>=3.10.2",