diff --git a/changelog.d/2260.changed.md b/changelog.d/2260.changed.md new file mode 100644 index 0000000000..6e75be16e9 --- /dev/null +++ b/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). diff --git a/policyengine_uk/simulation.py b/policyengine_uk/simulation.py index 5feee2bc38..bf21684c48 100644 --- a/policyengine_uk/simulation.py +++ b/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( diff --git a/policyengine_uk/tests/test_input_records.py b/policyengine_uk/tests/test_input_records.py new file mode 100644 index 0000000000..e80fa327dc --- /dev/null +++ b/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]} diff --git a/policyengine_uk/tests/test_ni_class_4_profits_properties.py b/policyengine_uk/tests/test_ni_class_4_profits_properties.py index 98a193d677..635c946d2e 100644 --- a/policyengine_uk/tests/test_ni_class_4_profits_properties.py +++ b/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") diff --git a/policyengine_uk/utils/supplied_inputs.py b/policyengine_uk/utils/supplied_inputs.py index 3e7d34caf7..39371fa0cc 100644 --- a/policyengine_uk/utils/supplied_inputs.py +++ b/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: diff --git a/pyproject.toml b/pyproject.toml index 1bdb2113f9..1b961ead87 100644 --- a/pyproject.toml +++ b/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", diff --git a/uv.lock b/uv.lock index e3fd3ac608..ebb3ff1a14 100644 --- a/uv.lock +++ b/uv.lock @@ -1453,7 +1453,7 @@ wheels = [ [[package]] name = "policyengine-core" -version = "3.32.13" +version = "3.32.27" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "dpath" }, @@ -1473,14 +1473,14 @@ dependencies = [ { name = "standard-imghdr" }, { name = "wheel" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/40/0a/b6c27953e1083d3ab2c40330c49580130fff48a2fe9cbfc8d731e92882e6/policyengine_core-3.32.13.tar.gz", hash = "sha256:a870f7e212fdfa1b5fdad7df92aa2b325fd0280ede16a40aeeceea74d44fb864", size = 457899, upload-time = "2026-10-03T11:30:20.572Z" } +sdist = { url = "https://files.pythonhosted.org/packages/de/24/84c0747c64fba95e89d3acd332604ad01e8a220ce7e80cf8d342b3b213bd/policyengine_core-3.32.27.tar.gz", hash = "sha256:632e8708ec243f7b592ec857e1978df207547c1510039373e707c310173cf4e4", size = 599334, upload-time = "2026-10-09T05:44:09.236Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/d8/01/00ebf4c13152347bc5396b40e80445fa92ea96712b9ec0bfdca7c5d11413/policyengine_core-3.32.13-py3-none-any.whl", hash = "sha256:262543c3ac696207f304f55664711b72e894fbad7b566407afa9d30b16086a41", size = 251251, upload-time = "2026-10-03T11:30:19.171Z" }, + { url = "https://files.pythonhosted.org/packages/cc/b3/fc1a10b03f2a159bc5623f0aa465887d1c1c98ee2f3118fd9ee3e8275921/policyengine_core-3.32.27-py3-none-any.whl", hash = "sha256:d0b0135939252ed628efd496988f76c0ab89bfa3b3437aa90f39000667c37687", size = 278496, upload-time = "2026-10-09T05:44:07.764Z" }, ] [[package]] name = "policyengine-uk" -version = "2.109.5" +version = "2.125.1" source = { editable = "." } dependencies = [ { name = "microdf-python" }, @@ -1514,7 +1514,7 @@ requires-dist = [ { name = "hypothesis", marker = "extra == 'dev'" }, { name = "jupyter-book", marker = "extra == 'dev'", specifier = ">=2.0.0a0" }, { name = "microdf-python", specifier = ">=1.2.1" }, - { name = "policyengine-core", specifier = ">=3.32.12" }, + { name = "policyengine-core", specifier = ">=3.32.27" }, { name = "pydantic", specifier = ">=2.11.7" }, { name = "pytest-cov", marker = "extra == 'dev'" }, { name = "pytest-xdist", marker = "extra == 'dev'" },