From b9009cff333cba8b4ce735522dfc1a10a6c72263 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 10 Oct 2026 11:28:16 -0400 Subject: [PATCH 1/3] Drop the clone and delete_arrays overrides now that core keeps the input record policyengine-core 3.32.27 contains PolicyEngine/policyengine-core#561, which fixes policyengine-core#559: Holder.delete_arrays forgets the record (Simulation._user_input_keys) of an input it deletes, including deletes made straight on the holder, and Simulation.clone gives each clone its own copy of the record and an empty _user_input_contexts. PE-UK's Simulation.clone and Simulation.delete_arrays overrides did the same by hand, so remove them and raise the policyengine-core floor from 3.32.12 to 3.32.27. Relock core 3.32.13 -> 3.32.27; the lock's own entry for this package catches up to 2.125.1. Keep the sweep in Simulation.calculate and SUPPLIED_INPUT_VARIABLES. Simulation.build_from_populations, which build_from_situation calls, replaces every holder but keeps the record (policyengine-core#605). A rebuilt simulation could then pass a carried-over trading loss off as supplied, which the sweep prevents. Its comment and the supplied_inputs docstring now name that case instead of #559. Tests: - a rebuild regression for Class 4 (fails with the sweep disabled); - a model-based property test: over random sequences of set_input, both kinds of delete, calculation, clones and rebuilds, a loss counts as supplied exactly while the last value set for it is kept. Two pinned examples make it fail on core 3.32.13 (one record shared by a clone) and with the sweep disabled (#605); - the missing-stored-array test now puts a stale record back itself, since core no longer leaves one after a holder delete. Co-Authored-By: Claude Opus 5.5 --- changelog.d/2260.changed.md | 1 + policyengine_uk/simulation.py | 34 +--- .../test_ni_class_4_profits_properties.py | 157 +++++++++++++++++- policyengine_uk/utils/supplied_inputs.py | 22 +-- pyproject.toml | 2 +- uv.lock | 10 +- 6 files changed, 175 insertions(+), 51 deletions(-) create mode 100644 changelog.d/2260.changed.md diff --git a/changelog.d/2260.changed.md b/changelog.d/2260.changed.md new file mode 100644 index 0000000000..865124ada5 --- /dev/null +++ b/changelog.d/2260.changed.md @@ -0,0 +1 @@ +- Require policyengine-core 3.32.27, which forgets the record of a `set_input` value that `Holder.delete_arrays` deletes and gives each clone its own record (PolicyEngine/policyengine-core#561), and remove the `Simulation.clone` and `Simulation.delete_arrays` overrides that worked around policyengine-core#559. `Simulation.calculate` still forgets stale records of the supplied-input variables, which rebuilding the populations leaves behind (PolicyEngine/policyengine-core#605). diff --git a/policyengine_uk/simulation.py b/policyengine_uk/simulation.py index 5feee2bc38..b768b7453b 100644 --- a/policyengine_uk/simulation.py +++ b/policyengine_uk/simulation.py @@ -9,7 +9,7 @@ # PolicyEngine core imports from policyengine_core.data import Dataset from policyengine_core.enums import Enum as CoreEnum -from policyengine_core.periods import Period, period as period_ +from policyengine_core.periods import period as period_ from policyengine_core.parameters import Parameter from policyengine_core.reforms import Reform from policyengine_core.simulations import Simulation as CoreSimulation @@ -235,30 +235,6 @@ 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. - # 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) - def reset_calculations(self): for variable in self.tax_benefit_system.variables: if variable not in self.input_variables: @@ -659,9 +635,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_ni_class_4_profits_properties.py b/policyengine_uk/tests/test_ni_class_4_profits_properties.py index 98a193d677..96782b0c6a 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,145 @@ 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) +# Each step names the simulation it acts on (modulo the number made so far), +# so an original keeps changing after it has been cloned. +lifecycle_step = st.tuples( + st.integers(min_value=0, max_value=3), + 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=6), +) +# 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..f539b8e114 100644 --- a/policyengine_uk/utils/supplied_inputs.py +++ b/policyengine_uk/utils/supplied_inputs.py @@ -4,14 +4,16 @@ 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 +(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. + +``Simulation.build_from_populations``, which ``build_from_situation`` calls, +replaces every holder but 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. @@ -60,7 +62,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 +114,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'" }, From 5f2ea22afd5d73e4367af01eae0374581744a9c2 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 10 Oct 2026 12:33:22 -0400 Subject: [PATCH 2/3] Keep the delete_arrays override for records a rebuild leaves behind Review round 2 found that the removed Simulation.delete_arrays override was not pure duplication of policyengine-core 3.32.27. Core's deletion forgets only the records of the values it removes. The override also dropped records that a population rebuild had already left with no stored value (policyengine-core#605), for any variable. Without it, deleting hours_worked for 2026 after a rebuild that dropped the 2026 input, then calculating 2026, made to_input_dict export the carried-over 2025 hours as a 2026 input (core 3.32.27 and 3.33.0). Restore the override, with its comment naming #605 instead of #559, and add test_input_records.py for that sequence (it fails without the override). The property test's step selector now reaches every simulation a run can make. Co-Authored-By: Claude Opus 5.5 --- changelog.d/2260.changed.md | 2 +- policyengine_uk/simulation.py | 13 +++++++- policyengine_uk/tests/test_input_records.py | 32 +++++++++++++++++++ .../test_ni_class_4_profits_properties.py | 10 +++--- policyengine_uk/utils/supplied_inputs.py | 2 ++ 5 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 policyengine_uk/tests/test_input_records.py diff --git a/changelog.d/2260.changed.md b/changelog.d/2260.changed.md index 865124ada5..6e75be16e9 100644 --- a/changelog.d/2260.changed.md +++ b/changelog.d/2260.changed.md @@ -1 +1 @@ -- Require policyengine-core 3.32.27, which forgets the record of a `set_input` value that `Holder.delete_arrays` deletes and gives each clone its own record (PolicyEngine/policyengine-core#561), and remove the `Simulation.clone` and `Simulation.delete_arrays` overrides that worked around policyengine-core#559. `Simulation.calculate` still forgets stale records of the supplied-input variables, which rebuilding the populations leaves behind (PolicyEngine/policyengine-core#605). +- 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 b768b7453b..bf21684c48 100644 --- a/policyengine_uk/simulation.py +++ b/policyengine_uk/simulation.py @@ -9,7 +9,7 @@ # PolicyEngine core imports from policyengine_core.data import Dataset from policyengine_core.enums import Enum as CoreEnum -from policyengine_core.periods import period as period_ +from policyengine_core.periods import Period, period as period_ from policyengine_core.parameters import Parameter from policyengine_core.reforms import Reform from policyengine_core.simulations import Simulation as CoreSimulation @@ -235,6 +235,17 @@ def __init__( if scenario.parameter_changes is not None: self.apply_parameter_changes(scenario.parameter_changes) + def delete_arrays(self, variable: str, period: Period = None) -> None: + super().delete_arrays(variable, period) + # 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) + def reset_calculations(self): for variable in self.tax_benefit_system.variables: if variable not in self.input_variables: 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 96782b0c6a..635c946d2e 100644 --- a/policyengine_uk/tests/test_ni_class_4_profits_properties.py +++ b/policyengine_uk/tests/test_ni_class_4_profits_properties.py @@ -828,10 +828,12 @@ def situation(losses): LIFECYCLE_YEARS = (2024, 2025, 2026) lifecycle_loss = st.integers(min_value=0, max_value=3).map(lambda k: 1_000.0 * k) -# Each step names the simulation it acts on (modulo the number made so far), -# so an original keeps changing after it has been cloned. +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=3), + 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( @@ -889,7 +891,7 @@ def assert_supplied_losses(sim, expected): @PROPERTY_SETTINGS @given( st.dictionaries(st.sampled_from(LIFECYCLE_YEARS), lifecycle_loss), - st.lists(lifecycle_step, max_size=6), + 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). diff --git a/policyengine_uk/utils/supplied_inputs.py b/policyengine_uk/utils/supplied_inputs.py index f539b8e114..2132617b66 100644 --- a/policyengine_uk/utils/supplied_inputs.py +++ b/policyengine_uk/utils/supplied_inputs.py @@ -17,6 +17,8 @@ ``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 From a25ce01794c86da416d0035335349c6b83dac688 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 10 Oct 2026 12:51:39 -0400 Subject: [PATCH 3/3] Say that build_from_situation installs the fresh populations Review round 3 (APPROVE) noted that Simulation.build_from_populations adopts whatever populations it is given: passed the existing ones, it keeps their holders. The holders are replaced because build_from_situation passes freshly instantiated populations. Word the supplied_inputs docstring that way. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/utils/supplied_inputs.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/policyengine_uk/utils/supplied_inputs.py b/policyengine_uk/utils/supplied_inputs.py index 2132617b66..39371fa0cc 100644 --- a/policyengine_uk/utils/supplied_inputs.py +++ b/policyengine_uk/utils/supplied_inputs.py @@ -10,11 +10,11 @@ clone gets its own copy of the record. The helpers also require a stored value, so missing arrays do not count as supplied inputs. -``Simulation.build_from_populations``, which ``build_from_situation`` calls, -replaces every holder but 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 +``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