From 819dc022d2fdc43cf1880c0675c0ae17768f9a1b Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 14 Oct 2026 02:28:28 -0400 Subject: [PATCH 1/5] Key the URL dataset cache on the uprating inputs, and let parameter changes that reach them re-uprate the data The cache held datasets already extended and uprated with whichever parameters loaded the URL first. A macro scenario built first left its uprated incomes for its own baseline and every later simulation; built second, it got current-law data. Datasets are now cached only when the parameters the uprating reads are current law's; any other simulation builds its own. A scenario applied after data load, which includes every reform dict, wrote its parameter changes after the data was built, so a changed growth rate moved the triple lock and the indices but not the incomes in the data. Changes that change what the uprating reads are now written before the data is built. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/data/economic_assumptions.py | 75 ++++++++++++- policyengine_uk/simulation.py | 110 ++++++++++++++++--- 2 files changed, 163 insertions(+), 22 deletions(-) diff --git a/policyengine_uk/data/economic_assumptions.py b/policyengine_uk/data/economic_assumptions.py index 8e0d7d4b73..c5d70372e3 100644 --- a/policyengine_uk/data/economic_assumptions.py +++ b/policyengine_uk/data/economic_assumptions.py @@ -4,6 +4,8 @@ CGT_INDEX_PATH, ) +import hashlib + import yaml from policyengine_core.parameters import ParameterNode from policyengine_core.simulations.simulation import _uprating_index_value @@ -103,6 +105,73 @@ def _unmapped_uprating_indices(uprating): } +COUNCIL_TAX_GROWTH = "gov.economic_assumptions.yoy_growth.obr.council_tax" +SOCIAL_RENT_GROWTH = "gov.economic_assumptions.yoy_growth.obr.social_rent" +PRIVATE_RENT_GROWTH = "gov.economic_assumptions.yoy_growth.ons.private_rental_prices" + + +def _uprating_index_map() -> dict: + with open(Path(__file__).parent / "uprating_indices.yaml", "r") as f: + return yaml.safe_load(f) + + +def uprating_input_paths() -> list: + """Every parameter path the dataset uprating reads. + + ``apply_single_year_uprating`` and the functions it calls read these, and + no other parameters: the growth rates in ``uprating_indices.yaml``, the + council tax and rent series, the indices of the input variables that + file leaves out (after ``LEGACY_PROJECTION_END_YEAR``) and the capital + gains projection's metadata (``CGT_INDEX_PATH``). A path may name a node + (the regional private rent series), which covers its parameters. + """ + uprating = _uprating_index_map() + series = {value for value in _COUNCIL_TAX_GROWTH_SERIES.values() if value} + return sorted( + set(uprating) + | {f"{COUNCIL_TAX_GROWTH}.{name}" for name in series} + | {SOCIAL_RENT_GROWTH, PRIVATE_RENT_GROWTH, CGT_INDEX_PATH} + | set(_unmapped_uprating_indices(uprating).values()) + ) + + +def uprating_input_fingerprint(parameters: ParameterNode) -> str: + """A digest of the parameter values the dataset uprating reads. + + Two parameter trees with the same fingerprint uprate a dataset + identically, so an uprated dataset can be shared between them. It covers + each read parameter's full timeline and the capital gains projection's + metadata. + """ + from policyengine_core.parameters import Parameter + + parts = [] + for path in uprating_input_paths(): + try: + node = parameters.get_child(path) + except ValueError: + parts.append((path, None)) + continue + leaves = ( + [node] + if isinstance(node, Parameter) + else [p for p in node.get_descendants() if isinstance(p, Parameter)] + ) + for leaf in sorted(leaves, key=lambda leaf: leaf.name): + parts.append( + ( + leaf.name, + tuple( + (entry.instant_str, repr(entry.value)) + for entry in leaf.values_list + ), + ) + ) + if path == CGT_INDEX_PATH: + parts.append((path, repr((node.metadata or {}).get("projection")))) + return hashlib.sha256(repr(parts).encode()).hexdigest() + + def extend_single_year_dataset( dataset: UKSingleYearDataset, tax_benefit_system_parameters: ParameterNode, @@ -184,8 +253,7 @@ def apply_single_year_uprating( # First, apply standard variable-YoY growth based uprating. - with open(Path(__file__).parent / "uprating_indices.yaml", "r") as f: - uprating = yaml.safe_load(f) + uprating = _uprating_index_map() for index_name, variables in uprating.items(): index_rel_change = parameters.get_child(index_name)(current_year.time_period) for variable in variables: @@ -455,8 +523,7 @@ def reset_uprating( def reset_growthfactor_uprating( dataset: UKMultiYearDataset, ): - with open(Path(__file__).parent / "uprating_indices.yaml", "r") as f: - uprating = yaml.safe_load(f) + uprating = _uprating_index_map() growthfactor_uprated_variables = { variable for variables in uprating.values() for variable in variables diff --git a/policyengine_uk/simulation.py b/policyengine_uk/simulation.py index 494463d9ca..4a7130d7f2 100644 --- a/policyengine_uk/simulation.py +++ b/policyengine_uk/simulation.py @@ -34,6 +34,7 @@ from policyengine_uk.data.economic_assumptions import ( extend_single_year_dataset, reset_growthfactor_uprating, + uprating_input_fingerprint, ) from policyengine_uk.data.dataset_sources import materialize_gcs_dataset_url from policyengine_uk.data.input_dataframe import ( @@ -67,6 +68,18 @@ from .tax_benefit_system import CountryTaxBenefitSystem, DEFAULT_DATASET_ENV_VAR +def _scenario_assignments(scenario) -> list: + """A scenario's statutory-timeline changes: ``parameter_changes``, then + each of its ``dated_reforms``, in order.""" + assignments = [] + if scenario.parameter_changes: + assignments += read_parameter_changes( + _canonical_parameter_paths(scenario.parameter_changes) + ) + assignments += scenario.dated_assignments(order_start=len(assignments)) + return assignments + + def _canonical_parameter_paths(changes: dict) -> dict: """Changes keyed by canonical parameter paths (labour supply aliases).""" return { @@ -81,6 +94,30 @@ def _canonical_parameter_paths(changes: dict) -> dict: # Avoids repeating HDF5 reading (0.84s), uprating (0.69s) and enum # encoding (0.67s) on every Simulation() call after the first. _url_dataset_cache: dict = {} +# The uprating fingerprint of current law, for each CGT projection selection +# (None: no projection), computed once. Only datasets uprated under current +# law are cached: a simulation whose uprating inputs differ builds its own. +_current_law_uprating_fingerprints: dict = {} + + +def _current_law_uprating_fingerprint(capital_gains_projection) -> str: + key = ( + None + if capital_gains_projection is None + else capital_gains_projection.fingerprint + ) + if key not in _current_law_uprating_fingerprints: + system = ( + CountryTaxBenefitSystem() + if capital_gains_projection is None + else CountryTaxBenefitSystem( + capital_gains_projection=capital_gains_projection + ) + ) + _current_law_uprating_fingerprints[key] = uprating_input_fingerprint( + system.parameters + ) + return _current_law_uprating_fingerprints[key] def get_default_dataset_url() -> str: @@ -212,9 +249,17 @@ def __init__( # A scenario's parameter changes rebuild the parameters from the # statutory tree, so they come before its modifier, whose edits a - # rebuild would discard. - if scenario is not None and scenario.applied_before_data_load: + # rebuild would discard. They come before the data is built when the + # scenario is applied before data load, and also when they change + # what the dataset's uprating reads (a growth rate, say), so the + # data's projected years follow them; otherwise just after. + parameters_before_data = scenario is not None and ( + scenario.applied_before_data_load + or self._scenario_changes_uprating_inputs(scenario) + ) + if parameters_before_data: self.apply_scenario_parameters(scenario) + if scenario is not None and scenario.applied_before_data_load: if scenario.simulation_modifier is not None: scenario.simulation_modifier(self) self._refresh_capital_gains_projection() @@ -236,12 +281,12 @@ def __init__( else: raise ValueError(f"Unsupported dataset type: {dataset.__class__}") - # Parameter changes applied after data load leave the data as built - # (a changed economic assumption does not re-uprate it), but come + # Parameter changes that leave the data's uprating alone come here, # before the steps below that read the parameters: the Universal # Credit rebalancing inputs and the structural reforms switched on by - # parameters. - if scenario is not None and not scenario.applied_before_data_load: + # parameters. Changes a scenario's modifier makes after data load + # (including single-day reform-dict keys) leave the data as built. + if scenario is not None and not parameters_before_data: self.apply_scenario_parameters(scenario) # Universal Credit reform (July 2025). Needs closer integration in the baseline, @@ -590,7 +635,20 @@ def capital_gains_projection_metadata(self) -> dict: } def _dataset_cache_key(self, source: str): - if self.tax_benefit_system.capital_gains_projection is None: + """The URL dataset cache key for a source, or None not to cache. + + A cached dataset is already extended and uprated, so it is shared + only between simulations whose parameters uprate it identically: + those whose uprating inputs are current law's. A simulation with + changed inputs (a macro scenario, ``no_economic_assumptions``, or a + reform that changes them) neither reads nor writes the cache. + """ + system = self.tax_benefit_system + if uprating_input_fingerprint( + system.parameters + ) != _current_law_uprating_fingerprint(system.capital_gains_projection): + return None + if system.capital_gains_projection is None: return source metadata = self.capital_gains_projection_metadata return ( @@ -632,13 +690,28 @@ def apply_scenario_parameters(self, scenario: Scenario) -> None: statutory tree does not define) are applied by the scenario's modifier instead. """ - assignments = [] - if scenario.parameter_changes: - assignments += read_parameter_changes( - _canonical_parameter_paths(scenario.parameter_changes) - ) - assignments += scenario.dated_assignments(order_start=len(assignments)) - self.apply_parameter_assignments(assignments) + self.apply_parameter_assignments(_scenario_assignments(scenario)) + + def _scenario_changes_uprating_inputs(self, scenario: Scenario) -> bool: + """Whether a scenario's parameter changes change the dataset uprating. + + Compares the uprating fingerprint of the parameters with the changes + (on a separate system, whose rebuild the memo then serves to this + one) with the fingerprint of the parameters without them. + """ + assignments = _scenario_assignments(scenario) + if not assignments: + return False + projection = self.tax_benefit_system.capital_gains_projection + probe = ( + CountryTaxBenefitSystem() + if projection is None + else CountryTaxBenefitSystem(capital_gains_projection=projection) + ) + probe.apply_parameter_assignments(assignments) + return uprating_input_fingerprint( + probe.parameters + ) != uprating_input_fingerprint(self.tax_benefit_system.parameters) def apply_parameter_assignments(self, assignments: list) -> None: """Rebuild the parameters with changes on the statutory timeline.""" @@ -682,7 +755,7 @@ def build_from_dataset_source( return if dataset_source.startswith("gs://"): cache_key = self._dataset_cache_key(dataset_source) - if cache_key in _url_dataset_cache: + if cache_key is not None and cache_key in _url_dataset_cache: multi_year_dataset = _url_dataset_cache[cache_key] self.build_from_multi_year_dataset(multi_year_dataset) self._restore_cgt_extension_provenance(multi_year_dataset) @@ -721,8 +794,9 @@ def build_from_file( # Pre-encode string enum columns to int16 once before caching so # subsequent loads skip the expensive astype(str) + searchsorted path. _pre_encode_enum_columns(multi_year_dataset, self.tax_benefit_system) - if cache_key is not None: - _url_dataset_cache[self._dataset_cache_key(cache_key)] = multi_year_dataset + key = self._dataset_cache_key(cache_key) if cache_key is not None else None + if key is not None: + _url_dataset_cache[key] = multi_year_dataset self.build_from_multi_year_dataset(multi_year_dataset) self._restore_cgt_extension_provenance(multi_year_dataset) @@ -743,7 +817,7 @@ def build_from_url(self, url: str) -> None: # Return early from in-memory cache if available: skips HDF5 reading, # uprating and enum encoding (~2.2s on the first load). cache_key = self._dataset_cache_key(url) - if cache_key in _url_dataset_cache: + if cache_key is not None and cache_key in _url_dataset_cache: multi_year_dataset = _url_dataset_cache[cache_key] self.build_from_multi_year_dataset(multi_year_dataset) self._restore_cgt_extension_provenance(multi_year_dataset) From 074e9e1b8c11ad4ea59660344dfd8e16b167dc0d Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 14 Oct 2026 02:37:00 -0400 Subject: [PATCH 2/5] Test that uprating inputs reach the data and the cache isolates them; document it MOCK datasets only. Both cache orders on hf:// and gs://, no economic assumptions, one shared extension under current law, reform dict versus the before-load route, the after-build timing of ordinary reforms, a property over growth rates and years, the fingerprint's sensitivity to every read input, and a read-recording guard that the fingerprint covers everything the extension reads. Co-Authored-By: Claude Opus 5.5 --- .../uprating-inputs-reach-data.fixed.md | 1 + docs/book/usage/scenarios.md | 2 +- .../tests/test_simulation_dataset_sources.py | 16 +- .../tests/test_uprating_inputs_reach_data.py | 369 ++++++++++++++++++ 4 files changed, 384 insertions(+), 4 deletions(-) create mode 100644 changelog.d/uprating-inputs-reach-data.fixed.md create mode 100644 policyengine_uk/tests/test_uprating_inputs_reach_data.py diff --git a/changelog.d/uprating-inputs-reach-data.fixed.md b/changelog.d/uprating-inputs-reach-data.fixed.md new file mode 100644 index 0000000000..50998d320b --- /dev/null +++ b/changelog.d/uprating-inputs-reach-data.fixed.md @@ -0,0 +1 @@ +Dataset caching no longer couples a macro scenario to other simulations: a scenario built first left its projected incomes for its own baseline and every later simulation from the same `hf://` or `gs://` dataset, and one built after a baseline got current law's data. Parameter changes that change what the dataset projection reads, such as a reform dict on an OBR growth rate, are now written before the data is built, so incomes, rents and council tax follow them. diff --git a/docs/book/usage/scenarios.md b/docs/book/usage/scenarios.md index d50098a3f0..ae24731f61 100644 --- a/docs/book/usage/scenarios.md +++ b/docs/book/usage/scenarios.md @@ -396,7 +396,7 @@ Each way of passing a reform reads period keys as it always has: A single-date key in a `reform=` dict overrides the annualised value on that day only. Formulas read annual values on 1 January, so `"2027-01-01"` sets fiscal year 2027 and `"2027-04-06"` changes nothing. The model logs a warning when a key can change nothing. Use `"start.stop"` keys for dated measures. -Changes applied after the data is loaded (the default) leave the data as built, so a changed economic assumption does not re-uprate incomes. Dated changes (`parameter_changes` and `"start.stop"` or bare-year keys) still come before the steps that read parameters while the simulation is built: the Universal Credit rebalancing awards and the structural reforms that parameters switch on. Single-day keys and a scenario's own simulation modifier run after those steps, as they always have. Set `applied_before_data_load=True` on a scenario to project the data with changed economic assumptions too. +A dataset's later years are projected from its first by the economic assumptions as the simulation is built. Dated changes that change what that projection reads (a growth rate, say, through `parameter_changes`, `"start.stop"` or bare-year keys) are written before the data is built, so incomes, rents and council tax follow them, as with `applied_before_data_load=True`. Other dated changes come just after the data is built, before the steps that read parameters while the simulation is built: the Universal Credit rebalancing awards and the structural reforms that parameters switch on. Single-day keys and a scenario's own simulation modifier run after those steps, as they always have, and leave the data as built. Datasets loaded from `hf://` or `gs://` are cached once projected, under current law only: a simulation whose projection inputs differ builds its own, so a scenario never reaches its baseline or a later simulation. All of this applies when the scenario or reform is passed to `Simulation(...)`. `Scenario.apply(simulation)` on a simulation that is already built edits its annualised values instead, as it always has: a `"2027-04-06.2028-04-05"` key there overrides those dates of the fiscal-year values, so it does not set fiscal year 2027, and the model logs a warning saying so. Pass the scenario when constructing the simulation for dated measures. diff --git a/policyengine_uk/tests/test_simulation_dataset_sources.py b/policyengine_uk/tests/test_simulation_dataset_sources.py index 630bca7657..a6a910b0a1 100644 --- a/policyengine_uk/tests/test_simulation_dataset_sources.py +++ b/policyengine_uk/tests/test_simulation_dataset_sources.py @@ -57,10 +57,20 @@ def fake_build_from_file(dataset_file, *, cache_key=None): assert captured == {"url": url} +def _current_law_system(): + """A stand-in system with current law's parameters: the URL cache is used + only by simulations whose dataset uprating inputs are current law's.""" + from policyengine_uk.tax_benefit_system import CountryTaxBenefitSystem + + return SimpleNamespace( + capital_gains_projection=None, parameters=CountryTaxBenefitSystem().parameters + ) + + def test_dataset_source_routes_gcs_urls_to_materialized_file(monkeypatch): captured = {} simulation = Simulation.__new__(Simulation) - simulation.tax_benefit_system = SimpleNamespace(capital_gains_projection=None) + simulation.tax_benefit_system = _current_law_system() url = "gs://policyengine-uk-data-private/enhanced_frs_2023_24.h5@1.55.10" def fake_materialize_gcs_dataset_url(dataset_url): @@ -91,7 +101,7 @@ def test_dataset_source_reuses_cached_gcs_dataset_before_materializing(monkeypat captured = {} cached_dataset = object() simulation = Simulation.__new__(Simulation) - simulation.tax_benefit_system = SimpleNamespace(capital_gains_projection=None) + simulation.tax_benefit_system = _current_law_system() url = "gs://policyengine-uk-data-private/enhanced_frs_2023_24.h5@1.55.10" def fake_materialize_gcs_dataset_url(dataset_url): @@ -161,7 +171,7 @@ def fake_build_from_multi_year_dataset(dataset): ) simulation = Simulation.__new__(Simulation) - simulation.tax_benefit_system = SimpleNamespace(capital_gains_projection=None) + simulation.tax_benefit_system = _current_law_system() monkeypatch.setattr( simulation, "build_from_multi_year_dataset", diff --git a/policyengine_uk/tests/test_uprating_inputs_reach_data.py b/policyengine_uk/tests/test_uprating_inputs_reach_data.py new file mode 100644 index 0000000000..7f05002487 --- /dev/null +++ b/policyengine_uk/tests/test_uprating_inputs_reach_data.py @@ -0,0 +1,369 @@ +"""Parameter changes that reach the dataset uprating reach the data. + +A single-year dataset is extended to later years and uprated from the +parameters as they stand when the data is built +(``policyengine_uk.data.economic_assumptions``). ``hf://`` and ``gs://`` +datasets are cached once extended. + +Invariants: + +1. Cache isolation: the cached dataset is uprated under current law. A + simulation whose uprating inputs differ (a macro scenario, + ``no_economic_assumptions``) neither reads nor writes it, so it gets its + own projection and every other simulation, its own baseline included, + gets current law's, in either order. +2. Current-law simulations still share one extension, ordinary reforms + included. +3. Data follows the parameters: a reform dict that changes a growth rate the + uprating reads projects the data by that rate in that year and by current + law's in every other year, exactly as the scenario applied before the data + load does; an ordinary reform leaves the data as current law's. +4. Fingerprint: equal for equal uprating inputs, different when any read + input changes, and covering every parameter the uprating reads (checked + by recording the reads). + +All datasets here are MOCK data: two synthetic households, not survey +records. +""" + +import pandas as pd +import pytest +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st +from policyengine_core.parameters import Parameter, ParameterNode + +import policyengine_uk.simulation as simulation_module +from policyengine_uk import Simulation +from policyengine_uk.data import economic_assumptions +from policyengine_uk.data.dataset_schema import UKSingleYearDataset +from policyengine_uk.data.economic_assumptions import ( + extend_single_year_dataset, + uprating_input_fingerprint, + uprating_input_paths, +) +from policyengine_uk.scenarios import no_economic_assumptions +from policyengine_uk.tax_benefit_system import CountryTaxBenefitSystem +from policyengine_uk.utils.scenario import Scenario + +AVERAGE_EARNINGS = "gov.economic_assumptions.yoy_growth.obr.average_earnings" +PERSONAL_ALLOWANCE = "gov.hmrc.income_tax.allowances.personal_allowance.amount" +HF_URL = "hf://policyengine/policyengine-uk-data-private/mock_frs_2025_26.h5" +GCS_URL = "gs://policyengine-uk-data-private/mock_frs_2025_26.h5" +CURRENT_LAW_2027 = 0.024 # yoy_growth.obr.average_earnings for 2027 + +PROPERTY_SETTINGS = settings( + max_examples=8, + deadline=None, + derandomize=True, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], +) + + +def mock_dataset() -> UKSingleYearDataset: + """MOCK data: two synthetic households with the columns uprating reads.""" + person = pd.DataFrame( + { + "person_id": [1, 2], + "person_benunit_id": [1, 2], + "person_household_id": [1, 2], + "age": [40, 70], + "employment_income": [30_000.0, 0.0], + } + ) + benunit = pd.DataFrame({"benunit_id": [1, 2]}) + household = pd.DataFrame( + { + "household_id": [1, 2], + "household_weight": [1.0, 1.0], + "region": ["LONDON", "WALES"], + "tenure_type": ["RENT_PRIVATELY", "OWNED_OUTRIGHT"], + "rent": [12_000.0, 0.0], + "council_tax": [1_500.0, 1_800.0], + } + ) + return UKSingleYearDataset( + person=person, benunit=benunit, household=household, fiscal_year=2025 + ) + + +def mock_dataset_with_every_uprated_input() -> UKSingleYearDataset: + """MOCK data carrying every variable the extension uprates. + + The indices of the variables ``uprating_indices.yaml`` leaves out are + read only for the variables a dataset has, so the read-set check needs + them all. + """ + from policyengine_uk.system import system + + dataset = mock_dataset() + tables = { + "person": dataset.person, + "benunit": dataset.benunit, + "household": dataset.household, + } + uprating = economic_assumptions._uprating_index_map() + names = set(economic_assumptions._unmapped_uprating_indices(uprating)) + names |= {name for variables in uprating.values() for name in variables} + for name in sorted(names): + variable = system.variables.get(name) + if variable is None or variable.value_type is not float: + continue + table = tables[variable.entity.key] + if name not in table.columns: + table[name] = 1.0 + return UKSingleYearDataset(**tables, fiscal_year=2025) + + +@pytest.fixture(scope="module") +def mock_path(tmp_path_factory): + path = tmp_path_factory.mktemp("mock") / "mock_frs_2025_26.h5" + mock_dataset().save(str(path)) + return str(path) + + +@pytest.fixture +def remote(monkeypatch, mock_path): + """Serve both URL schemes from the MOCK file, with an empty cache, and + count extensions.""" + monkeypatch.setattr(simulation_module, "_url_dataset_cache", {}) + monkeypatch.setattr( + simulation_module, "download_huggingface_dataset", lambda **_: mock_path + ) + monkeypatch.setattr( + simulation_module, "materialize_gcs_dataset_url", lambda url: mock_path + ) + calls = [] + + def counting_extend(dataset, parameters, *args, **kwargs): + calls.append(dataset.time_period) + return extend_single_year_dataset(dataset, parameters, *args, **kwargs) + + monkeypatch.setattr( + simulation_module, "extend_single_year_dataset", counting_extend + ) + return calls + + +def growth(simulation, year=2027): + """Employment income growth into ``year`` in the simulation's data.""" + data = simulation.dataset + return ( + data[year].person["employment_income"].sum() + / data[year - 1].person["employment_income"].sum() + - 1 + ) + + +def macro(rate=0.2, year=2027): + return Scenario( + parameter_changes={AVERAGE_EARNINGS: {f"year:{year}-01-01:1": rate}}, + applied_before_data_load=True, + ) + + +# 1. Cache isolation. + + +@pytest.mark.parametrize("url", [HF_URL, GCS_URL], ids=["hf", "gs"]) +@pytest.mark.parametrize( + "macro_first", [True, False], ids=["macro-first", "baseline-first"] +) +def test_a_macro_scenario_and_current_law_get_their_own_data(remote, url, macro_first): + if macro_first: + reformed = Simulation(dataset=url, scenario=macro()) + later = Simulation(dataset=url) + else: + later = Simulation(dataset=url) + reformed = Simulation(dataset=url, scenario=macro()) + assert growth(reformed) == pytest.approx(0.2) + assert growth(reformed.baseline) == pytest.approx(CURRENT_LAW_2027) + assert growth(later) == pytest.approx(CURRENT_LAW_2027) + # Only current law's projection is cached. + assert list(simulation_module._url_dataset_cache) == [url] + + +@pytest.mark.parametrize("no_assumptions_first", [True, False]) +def test_no_economic_assumptions_does_not_reach_later_simulations( + remote, no_assumptions_first +): + if no_assumptions_first: + frozen = Simulation(dataset=HF_URL, scenario=no_economic_assumptions) + later = Simulation(dataset=HF_URL) + else: + later = Simulation(dataset=HF_URL) + frozen = Simulation(dataset=HF_URL, scenario=no_economic_assumptions) + assert growth(frozen) == 0 + assert growth(later) == pytest.approx(CURRENT_LAW_2027) + + +# 2. Current-law simulations share one extension. + + +def test_current_law_and_ordinary_reforms_share_one_extension(remote): + first = Simulation(dataset=HF_URL) + second = Simulation(dataset=HF_URL) + reformed = Simulation(dataset=HF_URL, reform={PERSONAL_ALLOWANCE: 20_000}) + assert len(remote) == 1 + assert second.dataset is first.dataset + assert reformed.dataset is first.dataset + assert reformed.baseline.dataset is first.dataset + + +# 3. Data follows the parameters. + + +def all_years(simulation, column="employment_income"): + data = simulation.dataset + return {year: list(data[year].person[column]) for year in data.years} + + +def test_a_growth_reform_dict_projects_the_data_as_the_before_load_route(mock_path): + change = {AVERAGE_EARNINGS: {"year:2027-01-01:1": 0.2}} + by_dict = Simulation(dataset=mock_path, reform=change) + before_load = Simulation( + dataset=mock_path, + scenario=Scenario(parameter_changes=change, applied_before_data_load=True), + ) + assert growth(by_dict) == pytest.approx(0.2) + assert all_years(by_dict) == all_years(before_load) + calculated = ( + by_dict.calculate("employment_income", 2027).sum() + / by_dict.calculate("employment_income", 2026).sum() + ) + assert calculated == pytest.approx(1.2) + assert growth(by_dict.baseline) == pytest.approx(CURRENT_LAW_2027) + + +def test_a_macro_scenario_combined_with_a_reform_still_projects_the_data(mock_path): + # Scenario.__add__ makes the sum an after-load scenario. + combined = macro() + Scenario.from_reform({PERSONAL_ALLOWANCE: 20_000}) + simulation = Simulation(dataset=mock_path, scenario=combined) + assert growth(simulation) == pytest.approx(0.2) + assert ( + simulation.tax_benefit_system.parameters.get_child(PERSONAL_ALLOWANCE)( + "2027-01-01" + ) + == 20_000 + ) + + +def test_an_ordinary_reform_keeps_the_data_and_the_after_build_timing( + mock_path, monkeypatch +): + events = [] + build = Simulation.build_from_dataset_source + apply = Simulation.apply_scenario_parameters + + def recording_build(self, source): + events.append("build") + return build(self, source) + + def recording_apply(self, scenario): + events.append("parameters") + return apply(self, scenario) + + monkeypatch.setattr(Simulation, "build_from_dataset_source", recording_build) + monkeypatch.setattr(Simulation, "apply_scenario_parameters", recording_apply) + reformed = Simulation(dataset=mock_path, reform={PERSONAL_ALLOWANCE: 20_000}) + # The reform's own build, then its parameters, then its baseline's build. + assert events == ["build", "parameters", "build"] + assert all_years(reformed) == all_years(reformed.baseline) + + +@PROPERTY_SETTINGS +@given( + rate=st.integers(-50, 300).map(lambda n: n / 1000), + year=st.integers(2026, 2030), +) +def test_a_growth_reform_moves_the_data_in_its_year_only(mock_path, rate, year): + reformed = Simulation( + dataset=mock_path, + reform={AVERAGE_EARNINGS: {f"year:{year}-01-01:1": rate}}, + ) + for later in range(2026, 2033): + expected = rate if later == year else growth(reformed.baseline, later) + assert growth(reformed, later) == pytest.approx(expected, abs=1e-9) + + +# 4. Fingerprint. + + +def current_law_parameters(): + """A fresh system's parameters: processed, with the CGT projection index.""" + return CountryTaxBenefitSystem().parameters + + +def read_leaves(): + parameters = current_law_parameters() + leaves = [] + for path in uprating_input_paths(): + node = parameters.get_child(path) + if isinstance(node, Parameter): + leaves.append(path) + else: + leaves += [ + p.name for p in node.get_descendants() if isinstance(p, Parameter) + ] + return sorted(leaves) + + +READ_LEAVES = read_leaves() + + +@settings(max_examples=40, deadline=None, derandomize=True) +@given( + leaf=st.sampled_from(READ_LEAVES), + entry=st.integers(0, 10_000), + delta=st.sampled_from([1e-6, 0.01, -0.5]), +) +def test_the_fingerprint_changes_with_any_read_input(leaf, entry, delta): + current = uprating_input_fingerprint(current_law_parameters()) + parameters = current_law_parameters() + assert uprating_input_fingerprint(parameters) == current + parameter = parameters.get_child(leaf) + values = parameter.values_list + target = values[entry % len(values)] + target.value = (target.value or 0) + delta + assert uprating_input_fingerprint(parameters) != current + + +def test_the_fingerprint_covers_every_parameter_the_uprating_reads(monkeypatch): + read = set() + get_child = ParameterNode.get_child + parameter_at = Parameter._get_at_instant + node_at = ParameterNode._get_at_instant + + def recording_get_child(self, path): + read.add(f"{self.name}.{path}" if self.name else path) + return get_child(self, path) + + def recording_parameter_at(self, instant): + read.add(self.name) + return parameter_at(self, instant) + + def recording_node_at(self, instant): + read.add(self.name) + return node_at(self, instant) + + parameters = current_law_parameters() + monkeypatch.setattr(ParameterNode, "get_child", recording_get_child) + monkeypatch.setattr(Parameter, "_get_at_instant", recording_parameter_at) + monkeypatch.setattr(ParameterNode, "_get_at_instant", recording_node_at) + # Past the legacy horizon, so the unmapped indices are read too. + extend_single_year_dataset(mock_dataset_with_every_uprated_input(), parameters) + monkeypatch.undo() + + covered = uprating_input_paths() + # The unmapped indices, read past the legacy horizon, are in the set. + assert "gov.dft.rail.ridership_index" in read + for name in read: + assert any(name == path or name.startswith(f"{path}.") for path in covered), ( + name + ) + + +def test_the_read_set_names_inputs_outside_the_growth_rates(): + paths = uprating_input_paths() + assert "gov.dft.rail.ridership_index" in paths + assert economic_assumptions.CGT_INDEX_PATH in paths + assert AVERAGE_EARNINGS in paths From de78bb86b3e6c55ac4eaaf82762da2ce528d5832 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 14 Oct 2026 06:26:30 -0400 Subject: [PATCH 3/5] Skip the uprating check for sources that are not projected A situation, a DataFrame and a multi-year dataset are taken as given, so their scenarios keep the after-build timing without building a probe system to compare uprating inputs. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/simulation.py | 18 ++++++++++- .../tests/test_uprating_inputs_reach_data.py | 30 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/policyengine_uk/simulation.py b/policyengine_uk/simulation.py index 4a7130d7f2..21ece573d4 100644 --- a/policyengine_uk/simulation.py +++ b/policyengine_uk/simulation.py @@ -68,6 +68,19 @@ from .tax_benefit_system import CountryTaxBenefitSystem, DEFAULT_DATASET_ENV_VAR +def _projects_data(situation, dataset) -> bool: + """Whether building from this source projects a dataset's later years. + + ``extend_single_year_dataset`` projects a single-year dataset from the + parameters: one read from a file or URL (including the default dataset), + a ``UKSingleYearDataset`` or a policyengine-core ``Dataset``. A situation, + a DataFrame and a multi-year dataset are taken as given. + """ + if situation is not None: + return False + return not isinstance(dataset, (pd.DataFrame, UKMultiYearDataset)) + + def _scenario_assignments(scenario) -> list: """A scenario's statutory-timeline changes: ``parameter_changes``, then each of its ``dated_reforms``, in order.""" @@ -255,7 +268,10 @@ def __init__( # data's projected years follow them; otherwise just after. parameters_before_data = scenario is not None and ( scenario.applied_before_data_load - or self._scenario_changes_uprating_inputs(scenario) + or ( + _projects_data(situation, dataset) + and self._scenario_changes_uprating_inputs(scenario) + ) ) if parameters_before_data: self.apply_scenario_parameters(scenario) diff --git a/policyengine_uk/tests/test_uprating_inputs_reach_data.py b/policyengine_uk/tests/test_uprating_inputs_reach_data.py index 7f05002487..ca601091fa 100644 --- a/policyengine_uk/tests/test_uprating_inputs_reach_data.py +++ b/policyengine_uk/tests/test_uprating_inputs_reach_data.py @@ -270,6 +270,36 @@ def recording_apply(self, scenario): assert all_years(reformed) == all_years(reformed.baseline) +@pytest.mark.parametrize("source", ["situation", "dataframe"]) +def test_sources_that_are_not_projected_skip_the_uprating_check( + mock_path, monkeypatch, source +): + checks = [] + monkeypatch.setattr( + Simulation, + "_scenario_changes_uprating_inputs", + lambda self, scenario: checks.append(scenario) or True, + ) + change = {AVERAGE_EARNINGS: {"year:2027-01-01:1": 0.2}} + if source == "situation": + situation = { + "people": { + "adult": {"age": {2025: 40}, "employment_income": {2025: 30_000}} + }, + "benunits": {"benunit": {"members": ["adult"]}}, + "households": {"household": {"members": ["adult"]}}, + } + reformed = Simulation(situation=situation, reform=change) + else: + frame = Simulation(dataset=mock_path).to_input_dataframe() + reformed = Simulation(dataset=frame, reform=change) + assert checks == [] + # The reform still reaches the parameters. + assert reformed.tax_benefit_system.parameters.get_child(AVERAGE_EARNINGS)( + "2027-01-01" + ) == pytest.approx(0.2) + + @PROPERTY_SETTINGS @given( rate=st.integers(-50, 300).map(lambda n: n / 1000), From 3eaa753897358a0b1c855f13798e52efd7fb2837 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 14 Oct 2026 09:28:28 -0400 Subject: [PATCH 4/5] Address review: build no system at collection, probe on a copy of the system The fingerprint tests built a tax-benefit system while the tests were collected, which the collection-lifetime check rejects; they now read the module's system. The uprating probe rebuilt parameters on a whole new system, loading every variable; it now rebuilds them on a shallow copy of the simulation's own system (CountryTaxBenefitSystem.parameters_with). Co-Authored-By: Claude Opus 5.5 --- policyengine_uk/simulation.py | 16 ++++------ policyengine_uk/tax_benefit_system.py | 12 ++++++++ .../tests/test_simulation_dataset_sources.py | 6 ++-- .../tests/test_uprating_inputs_reach_data.py | 30 +++++++++++++++++-- 4 files changed, 46 insertions(+), 18 deletions(-) diff --git a/policyengine_uk/simulation.py b/policyengine_uk/simulation.py index 21ece573d4..e940b28977 100644 --- a/policyengine_uk/simulation.py +++ b/policyengine_uk/simulation.py @@ -712,22 +712,16 @@ def _scenario_changes_uprating_inputs(self, scenario: Scenario) -> bool: """Whether a scenario's parameter changes change the dataset uprating. Compares the uprating fingerprint of the parameters with the changes - (on a separate system, whose rebuild the memo then serves to this - one) with the fingerprint of the parameters without them. + (rebuilt on a copy of the system, a rebuild the memo then serves to + this one) with that of the parameters without them. """ assignments = _scenario_assignments(scenario) if not assignments: return False - projection = self.tax_benefit_system.capital_gains_projection - probe = ( - CountryTaxBenefitSystem() - if projection is None - else CountryTaxBenefitSystem(capital_gains_projection=projection) - ) - probe.apply_parameter_assignments(assignments) + system = self.tax_benefit_system return uprating_input_fingerprint( - probe.parameters - ) != uprating_input_fingerprint(self.tax_benefit_system.parameters) + system.parameters_with(assignments) + ) != uprating_input_fingerprint(system.parameters) def apply_parameter_assignments(self, assignments: list) -> None: """Rebuild the parameters with changes on the statutory timeline.""" diff --git a/policyengine_uk/tax_benefit_system.py b/policyengine_uk/tax_benefit_system.py index 14e58f88c7..af4ed28b51 100644 --- a/policyengine_uk/tax_benefit_system.py +++ b/policyengine_uk/tax_benefit_system.py @@ -312,6 +312,18 @@ def apply_parameter_assignments( finally: self.reset_parameter_caches() + def parameters_with(self, assignments: List[ParameterAssignment]) -> ParameterNode: + """The parameters this system would have with more dated changes. + + Rebuilds them on a shallow copy, which shares the variables and + entities, so this system is left as it is. A rebuild only reassigns + the copy's attributes, and the memo then serves the same rebuild to + this system if it is given the changes. + """ + probe = copy.copy(self) + probe.apply_parameter_assignments(assignments) + return probe.parameters + def add_legacy_overrides(self, overrides: List[LegacyOverride]) -> None: """Apply changes to the annualised parameters and keep them. diff --git a/policyengine_uk/tests/test_simulation_dataset_sources.py b/policyengine_uk/tests/test_simulation_dataset_sources.py index a6a910b0a1..278d0bbc4d 100644 --- a/policyengine_uk/tests/test_simulation_dataset_sources.py +++ b/policyengine_uk/tests/test_simulation_dataset_sources.py @@ -60,11 +60,9 @@ def fake_build_from_file(dataset_file, *, cache_key=None): def _current_law_system(): """A stand-in system with current law's parameters: the URL cache is used only by simulations whose dataset uprating inputs are current law's.""" - from policyengine_uk.tax_benefit_system import CountryTaxBenefitSystem + from policyengine_uk.tax_benefit_system import system - return SimpleNamespace( - capital_gains_projection=None, parameters=CountryTaxBenefitSystem().parameters - ) + return SimpleNamespace(capital_gains_projection=None, parameters=system.parameters) def test_dataset_source_routes_gcs_urls_to_materialized_file(monkeypatch): diff --git a/policyengine_uk/tests/test_uprating_inputs_reach_data.py b/policyengine_uk/tests/test_uprating_inputs_reach_data.py index ca601091fa..757925e8b3 100644 --- a/policyengine_uk/tests/test_uprating_inputs_reach_data.py +++ b/policyengine_uk/tests/test_uprating_inputs_reach_data.py @@ -319,12 +319,19 @@ def test_a_growth_reform_moves_the_data_in_its_year_only(mock_path, rate, year): def current_law_parameters(): - """A fresh system's parameters: processed, with the CGT projection index.""" - return CountryTaxBenefitSystem().parameters + """A copy of current law's processed parameters, CGT projection index + included. Copied from the module's system: building a system while the + tests are collected would leave its variable modules frozen + (``test_variable_module_release``).""" + from policyengine_uk.tax_benefit_system import system + + return system.parameters.clone() def read_leaves(): - parameters = current_law_parameters() + from policyengine_uk.tax_benefit_system import system + + parameters = system.parameters leaves = [] for path in uprating_input_paths(): node = parameters.get_child(path) @@ -397,3 +404,20 @@ def test_the_read_set_names_inputs_outside_the_growth_rates(): assert "gov.dft.rail.ridership_index" in paths assert economic_assumptions.CGT_INDEX_PATH in paths assert AVERAGE_EARNINGS in paths + + +def test_the_probe_leaves_the_system_as_it_was(): + from policyengine_uk.utils.scenario import Scenario as S + + simulation = Simulation.__new__(Simulation) + simulation.tax_benefit_system = CountryTaxBenefitSystem() + system = simulation.tax_benefit_system + before = uprating_input_fingerprint(system.parameters) + parameters = system.parameters + change = S(parameter_changes={AVERAGE_EARNINGS: {"year:2027-01-01:1": 0.2}}) + assert simulation._scenario_changes_uprating_inputs(change) + assert system.parameters is parameters + assert system.dated_assignments == [] + assert uprating_input_fingerprint(system.parameters) == before + ordinary = S(parameter_changes={PERSONAL_ALLOWANCE: 20_000}) + assert not simulation._scenario_changes_uprating_inputs(ordinary) From a84a386d781f5d4beb60b05748f17b51ffa6e64c Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 14 Oct 2026 15:14:31 -0400 Subject: [PATCH 5/5] Address review: the probe keeps the system's caches, even when it fails parameters_with rebuilt on a shallow copy that shared the system's tree, so a failing rebuild restored that tree onto the copy and its cache reset emptied the system's node caches. The copy now holds its own clone of the tree, so the system is untouched either way and the tree returned is never the system's own. The docstring says when the memo serves the real application, and the changelog names dated parameter changes. Co-Authored-By: Claude Opus 5.5 --- .../uprating-inputs-reach-data.fixed.md | 2 +- policyengine_uk/tax_benefit_system.py | 11 ++++++--- .../tests/test_uprating_inputs_reach_data.py | 23 +++++++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/changelog.d/uprating-inputs-reach-data.fixed.md b/changelog.d/uprating-inputs-reach-data.fixed.md index 50998d320b..588bc964b8 100644 --- a/changelog.d/uprating-inputs-reach-data.fixed.md +++ b/changelog.d/uprating-inputs-reach-data.fixed.md @@ -1 +1 @@ -Dataset caching no longer couples a macro scenario to other simulations: a scenario built first left its projected incomes for its own baseline and every later simulation from the same `hf://` or `gs://` dataset, and one built after a baseline got current law's data. Parameter changes that change what the dataset projection reads, such as a reform dict on an OBR growth rate, are now written before the data is built, so incomes, rents and council tax follow them. +Dataset caching no longer couples a macro scenario to other simulations: a scenario built first left its projected incomes for its own baseline and every later simulation from the same `hf://` or `gs://` dataset, and one built after a baseline got current law's data. Dated parameter changes that change what the dataset projection reads, such as a reform dict setting an OBR growth rate for a year, are now written before the data is built, so incomes, rents and council tax follow them. diff --git a/policyengine_uk/tax_benefit_system.py b/policyengine_uk/tax_benefit_system.py index af4ed28b51..504e43b8b1 100644 --- a/policyengine_uk/tax_benefit_system.py +++ b/policyengine_uk/tax_benefit_system.py @@ -316,11 +316,16 @@ def parameters_with(self, assignments: List[ParameterAssignment]) -> ParameterNo """The parameters this system would have with more dated changes. Rebuilds them on a shallow copy, which shares the variables and - entities, so this system is left as it is. A rebuild only reassigns - the copy's attributes, and the memo then serves the same rebuild to - this system if it is given the changes. + entities but holds its own copy of the parameters, so this system is + left as it is, its parameter caches included, even when the rebuild + fails. A rebuild only reassigns the copy's attributes. When this + system was processed from the YAML and the changes' values can be + memoised (plain numbers, booleans and strings), the memo then serves + the same rebuild to this system if it is given the changes. The tree + returned is never this system's own. """ probe = copy.copy(self) + probe.parameters = self.parameters.clone() probe.apply_parameter_assignments(assignments) return probe.parameters diff --git a/policyengine_uk/tests/test_uprating_inputs_reach_data.py b/policyengine_uk/tests/test_uprating_inputs_reach_data.py index 757925e8b3..9f7143e2be 100644 --- a/policyengine_uk/tests/test_uprating_inputs_reach_data.py +++ b/policyengine_uk/tests/test_uprating_inputs_reach_data.py @@ -421,3 +421,26 @@ def test_the_probe_leaves_the_system_as_it_was(): assert uprating_input_fingerprint(system.parameters) == before ordinary = S(parameter_changes={PERSONAL_ALLOWANCE: 20_000}) assert not simulation._scenario_changes_uprating_inputs(ordinary) + + +def test_a_failing_probe_leaves_the_system_and_its_caches_alone(): + from policyengine_uk.utils.dated_reforms import ParameterAssignment + from policyengine_core.periods import instant + + system = CountryTaxBenefitSystem() + parameters = system.parameters + # Fill some node caches. + parameters("2027-01-01") + filled = [ + n for n in parameters.get_descendants() if getattr(n, "_at_instant_cache", None) + ] + assert filled + bad = ParameterAssignment( + "gov.not.a.parameter", instant("2027-01-01"), instant("2027-12-31"), 1 + ) + with pytest.raises(ValueError, match="Could not find the parameter"): + system.parameters_with([bad]) + assert system.parameters is parameters + assert all(getattr(n, "_at_instant_cache", None) for n in filled) + # With no changes, the tree returned is a copy, not the system's own. + assert system.parameters_with([]) is not system.parameters