From c825cab722bf202f1815e63e63e245fe7b1b3ff2 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Fri, 2 Oct 2026 07:41:35 -0400 Subject: [PATCH] Drop every contained period from the fast cache, as storage does Holder storage deletes every period the deleted one contains (deleting "2012" also deletes "2012-02"), and an ETERNITY variable keeps one value for every period. Simulation.delete_arrays, set_input and purge_cache_of_invalid_values popped only the exact (variable, period) key from _fast_cache, so calculate kept returning values storage had deleted or replaced: a month after its year was deleted, an ETERNITY value set or deleted at another period, months of an annual input split by a set_input handler, and months of a spiral-invalidated year. _evict_fast_cache drops each of the variable's entries whose period the given period contains, using the same Period.contains test as InMemoryStorage.delete, and all of them for ETERNITY variables or no period. Found by the adversarial review of #562 (GPT-6.1 Sol, 2026-10-01); reproduces on master 7950c01f and b78b0ba9. Co-Authored-By: Claude Opus 5.5 --- .../fast-cache-contained-periods.fixed.md | 1 + policyengine_core/simulations/simulation.py | 60 ++++-- .../core/test_fast_cache_contained_periods.py | 133 +++++++++++++ ...t_fast_cache_contained_periods_property.py | 176 ++++++++++++++++++ .../fixtures/fast_cache_contained_periods.py | 77 ++++++++ 5 files changed, 431 insertions(+), 16 deletions(-) create mode 100644 changelog.d/fast-cache-contained-periods.fixed.md create mode 100644 tests/core/test_fast_cache_contained_periods.py create mode 100644 tests/core/test_fast_cache_contained_periods_property.py create mode 100644 tests/fixtures/fast_cache_contained_periods.py diff --git a/changelog.d/fast-cache-contained-periods.fixed.md b/changelog.d/fast-cache-contained-periods.fixed.md new file mode 100644 index 000000000..8962c68b4 --- /dev/null +++ b/changelog.d/fast-cache-contained-periods.fixed.md @@ -0,0 +1 @@ +`delete_arrays`, `set_input` and spiral-cache purging now drop every contained period from the simulation's fast cache (every period for ETERNITY variables), as holder storage does, so `calculate` no longer returns values storage has deleted or replaced. diff --git a/policyengine_core/simulations/simulation.py b/policyengine_core/simulations/simulation.py index 2652d9723..bd18933bf 100644 --- a/policyengine_core/simulations/simulation.py +++ b/policyengine_core/simulations/simulation.py @@ -1007,9 +1007,43 @@ def purge_cache_of_invalid_values(self) -> None: holder = self.get_holder(_name) holder.delete_arrays(_period) if _fast_cache is not None: - _fast_cache.pop((_name, _period), None) + self._evict_fast_cache(_name, _period) self.invalidated_caches = set() + def _evict_fast_cache(self, variable_name: str, period: Period = None) -> None: + """Drop the ``_fast_cache`` entries a storage write or delete at ``period`` makes stale. + + Holder storage deletes (and ``set_input`` handlers write) every period + that ``period`` contains: deleting ``"2017"`` also deletes + ``"2017-01"`` to ``"2017-12"``, and an ETERNITY variable keeps one + value whatever period it is asked for. The fast cache is keyed by the + period ``calculate`` was asked for, so this drops each of the + variable's entries whose period ``period`` contains, and all of them + when ``period`` is ``None`` or the variable is defined for ETERNITY. + Popping only ``(variable_name, period)`` left ``calculate`` returning + values for contained periods that storage no longer held. + """ + _fast_cache = getattr(self, "_fast_cache", None) + if not _fast_cache: + return + if period is not None: + period = periods.period(period) + variable = self.tax_benefit_system.get_variable(variable_name) + if getattr(variable, "definition_period", None) == periods.ETERNITY: + period = None + stale_keys = [ + key + for key in _fast_cache + if key[0] == variable_name + and ( + period is None + or not isinstance(key[1], Period) + or period.contains(key[1]) + ) + ] + for key in stale_keys: + del _fast_cache[key] + def calculate_add( self, variable_name: str, @@ -1356,7 +1390,9 @@ def delete_arrays(self, variable: str, period: Period = None) -> None: names and the parent simulation's holder storage remain unchanged. :param variable: the variable whose cached values should be deleted - :param period: the period to delete, or all periods when omitted + :param period: the period to delete, with every period it contains + ("2018" also deletes "2018-05"; any period deletes an ETERNITY + variable's value), or all periods when omitted Example: @@ -1381,17 +1417,9 @@ def delete_arrays(self, variable: str, period: Period = None) -> None: holder = self.get_holder(variable) for branch_name in self._get_visible_branch_names(): holder.delete_arrays(period, branch_name) - _fast_cache = getattr(self, "_fast_cache", None) - if period is None: - if _fast_cache is not None: - self._fast_cache = { - k: v for k, v in _fast_cache.items() if k[0] != variable - } - else: - if not isinstance(period, Period): - period = periods.period(period) - if _fast_cache is not None: - _fast_cache.pop((variable, period), None) + # Same periods as the holder deletes: deleting "2017" also drops a + # cached "2017-02". + self._evict_fast_cache(variable, period) def get_known_periods(self, variable: str) -> List[Period]: """ @@ -1436,9 +1464,9 @@ def set_input(self, variable_name: str, period: Period, value: ArrayLike) -> Non if (variable.end is not None) and (period.start.date > variable.end): return self.get_holder(variable_name).set_input(period, value, self.branch_name) - _fast_cache = getattr(self, "_fast_cache", None) - if _fast_cache is not None: - _fast_cache.pop((variable_name, period), None) + # A ``set_input`` handler may store every period ``period`` contains + # (an annual input to a monthly variable stores twelve months). + self._evict_fast_cache(variable_name, period) def get_variable_population(self, variable_name: str) -> Population: variable = self.tax_benefit_system.get_variable( diff --git a/tests/core/test_fast_cache_contained_periods.py b/tests/core/test_fast_cache_contained_periods.py new file mode 100644 index 000000000..23b0c2f55 --- /dev/null +++ b/tests/core/test_fast_cache_contained_periods.py @@ -0,0 +1,133 @@ +"""Deleting or setting a period drops every period it contains from the fast cache. + +Holder storage deletes each period the requested one contains (deleting +"2012" also deletes "2012-02"), and an ETERNITY variable keeps one value for +every period. ``Simulation._fast_cache`` used to drop only the exact +``(variable, period)`` key, so ``calculate`` kept returning values storage no +longer held. The property versions of these tests are in +``test_fast_cache_contained_periods_property.py``. +""" + +from __future__ import annotations + +import numpy as np + +from policyengine_core import periods +from tests.fixtures.fast_cache_contained_periods import build_simulation + + +def test_deleting_a_year_drops_cached_months(): + """The carry-over review's repro: a deleted month must not come back from the cache.""" + simulation = build_simulation() + simulation.set_input("monthly_input", "2012-01", [100]) + np.testing.assert_array_equal( + simulation.calculate("monthly_input", "2012-02"), [100] + ) + + simulation.delete_arrays("monthly_input", "2012") + + assert simulation.get_array("monthly_input", "2012-01") is None + assert simulation.get_array("monthly_input", "2012-02") is None + simulation.set_input("monthly_input", "2012-01", [200]) + fresh = build_simulation() + fresh.set_input("monthly_input", "2012-01", [200]) + np.testing.assert_array_equal( + simulation.calculate("monthly_input", "2012-02"), + fresh.calculate("monthly_input", "2012-02"), + ) + np.testing.assert_array_equal( + simulation.calculate("monthly_input", "2012-02"), [200] + ) + + +def test_deleting_a_year_keeps_cached_values_it_does_not_contain(): + """Only what storage deletes leaves the fast cache.""" + simulation = build_simulation() + simulation.set_input("monthly_input", "2012-01", [100]) + february = simulation.calculate("monthly_input", "2012-02") + january_next_year = simulation.calculate("monthly_input", "2013-01") + other_variable = simulation.calculate("monthly_formula", "2012-02") + + simulation.delete_arrays("monthly_input", "2012") + + cache = simulation._fast_cache + assert ("monthly_input", periods.period("2012-02")) not in cache + assert cache[("monthly_input", periods.period("2013-01"))] is january_next_year + assert cache[("monthly_formula", periods.period("2012-02"))] is other_variable + assert february is not None + + +def test_deleting_a_month_keeps_the_rest_of_the_year(): + simulation = build_simulation() + simulation.set_input("monthly_input", "2012-01", [100]) + simulation.calculate("monthly_input", "2012-02") + march = simulation.calculate("monthly_input", "2012-03") + + simulation.delete_arrays("monthly_input", "2012-02") + + assert ("monthly_input", periods.period("2012-02")) not in simulation._fast_cache + assert simulation._fast_cache[("monthly_input", periods.period("2012-03"))] is march + + +def test_deleting_an_eternity_variable_at_any_period_drops_every_cached_period(): + """Storage keeps one ETERNITY value, so deleting it at "2012" also clears "2013".""" + simulation = build_simulation(carry_over=False) + simulation.set_input("eternal_input", "2012", [5]) + np.testing.assert_array_equal(simulation.calculate("eternal_formula", "2013"), [10]) + + simulation.delete_arrays("eternal_formula", "2012") + simulation.set_input("eternal_input", "2012", [7]) + + assert simulation.get_array("eternal_formula", "2013") is None + np.testing.assert_array_equal(simulation.calculate("eternal_formula", "2013"), [14]) + + +def test_setting_an_eternity_input_at_one_period_drops_it_at_every_period(): + simulation = build_simulation(carry_over=False) + np.testing.assert_array_equal(simulation.calculate("eternal_input", "2012"), [0]) + + simulation.set_input("eternal_input", "2013", [5]) + + np.testing.assert_array_equal(simulation.get_array("eternal_input", "2012"), [5]) + np.testing.assert_array_equal(simulation.calculate("eternal_input", "2012"), [5]) + + +def test_setting_a_year_split_over_months_drops_cached_months(): + """A ``set_input`` handler stores every month of an annual input. + + A variable the cache blacklist keeps out of storage still has its + calculated months in the fast cache, so those must go too. + """ + simulation = build_simulation(carry_over=False) + simulation.opt_out_cache = True + simulation.tax_benefit_system.cache_blacklist = {"monthly_input_split"} + np.testing.assert_array_equal( + simulation.calculate("monthly_input_split", "2012-02"), [0] + ) + + simulation.set_input("monthly_input_split", "2012", [1200]) + + np.testing.assert_array_equal( + simulation.get_array("monthly_input_split", "2012-02"), [100] + ) + np.testing.assert_array_equal( + simulation.calculate("monthly_input_split", "2012-02"), [100] + ) + + +def test_purging_an_invalidated_year_drops_cached_months(): + """``purge_cache_of_invalid_values`` deletes like ``delete_arrays`` does.""" + simulation = build_simulation(carry_over=False) + simulation.set_input("monthly_input", "2012-02", [10]) + np.testing.assert_array_equal( + simulation.calculate("monthly_formula", "2012-02"), [11] + ) + + simulation.invalidated_caches.add(("monthly_formula", periods.period("2012"))) + simulation.purge_cache_of_invalid_values() + simulation.set_input("monthly_input", "2012-02", [20]) + + assert simulation.get_array("monthly_formula", "2012-02") is None + np.testing.assert_array_equal( + simulation.calculate("monthly_formula", "2012-02"), [21] + ) diff --git a/tests/core/test_fast_cache_contained_periods_property.py b/tests/core/test_fast_cache_contained_periods_property.py new file mode 100644 index 000000000..26e11ef76 --- /dev/null +++ b/tests/core/test_fast_cache_contained_periods_property.py @@ -0,0 +1,176 @@ +"""Properties of the fast cache when periods are deleted or set. + +Invariants (each tested for random inputs): + +1. The fast cache never changes what ``calculate`` returns. Random sequences + of ``set_input``, ``calculate`` and ``delete_arrays`` run on two + simulations, one of which empties its fast cache before every + ``calculate`` (so every value comes from storage or a formula). Every + result, and every stored value at the end, must agree. +2. ``delete_arrays`` drops from the fast cache exactly the periods it drops + from holder storage: afterwards the variable's fast-cache periods equal its + stored periods, whatever was in both before, and other variables' entries + are untouched. + +``test_fast_cache_contained_periods.py`` pins the same behaviour with +examples. +""" + +from __future__ import annotations + +import numpy as np +import pytest + +# The country-package smoke job installs no dev dependencies. +hypothesis = pytest.importorskip("hypothesis") +st = hypothesis.strategies + +from policyengine_core import periods +from tests.fixtures.fast_cache_contained_periods import build_simulation + +MONTHS = ["2012-01", "2012-02", "2012-12", "2013-01", "2013-02"] +YEARS = ["2012", "2013"] +ANY_PERIOD = MONTHS + YEARS + +# (variable, periods ``set_input`` may use) +SET_INPUTS = [ + ("monthly_input", MONTHS), + ("monthly_input_split", MONTHS + YEARS), + ("yearly_input", YEARS), + ("eternal_input", ANY_PERIOD), +] +# (variable, periods ``calculate`` may use); a monthly variable read for a +# year is summed, a yearly one read for a month is divided. +READS = [ + ("monthly_input", ANY_PERIOD), + ("monthly_input_split", ANY_PERIOD), + ("monthly_formula", ANY_PERIOD), + ("yearly_input", ANY_PERIOD), + ("eternal_input", ANY_PERIOD), + ("eternal_formula", ANY_PERIOD), +] +DELETE_PERIODS = [None, periods.ETERNITY] + ANY_PERIOD + +_value = st.sampled_from([0.0, 1.5, 100.0, 1200.0]) + + +def _operation_over(variables): + """``set_input``, ``calculate`` or ``delete_arrays`` on one of ``variables``.""" + set_inputs = [(name, ps) for name, ps in SET_INPUTS if name in variables] + reads = [(name, ps) for name, ps in READS if name in variables] + options = [ + st.sampled_from(reads).flatmap( + lambda pair: st.tuples( + st.just("calculate"), st.just(pair[0]), st.sampled_from(pair[1]) + ) + ), + st.tuples( + st.just("delete_arrays"), + st.sampled_from([name for name, _ in reads]), + st.sampled_from(DELETE_PERIODS), + ), + ] + if set_inputs: + options.append( + st.sampled_from(set_inputs).flatmap( + lambda pair: st.tuples( + st.just("set_input"), + st.just(pair[0]), + st.sampled_from(pair[1]), + _value, + ) + ) + ) + return st.one_of(*options) + + +# Sequences over every variable, or over one family (an input and what is +# calculated from it), where the same values are hit again more often. +FAMILIES = [ + ("monthly_input", "monthly_formula"), + ("monthly_input_split",), + ("yearly_input",), + ("eternal_input", "eternal_formula"), +] +_operations = st.one_of( + st.lists(_operation_over([name for name, _ in READS]), max_size=25), + st.sampled_from(FAMILIES).flatmap( + lambda family: st.lists(_operation_over(family), max_size=15) + ), +) + + +def _result(call): + """What a call returns, or the type of error it raises.""" + try: + value = call() + except Exception as error: # Compared across simulations, not hidden. + return ("raised", type(error).__name__) + if value is None: + return ("none",) + return ("value", np.asarray(value).tolist()) + + +def _apply(simulation, operation, empty_fast_cache_first=False): + kind, variable, period, *rest = operation + if kind == "set_input": + return _result(lambda: simulation.set_input(variable, period, [rest[0]])) + if kind == "calculate": + if empty_fast_cache_first: + simulation._fast_cache.clear() + return _result(lambda: simulation.calculate(variable, period)) + return _result(lambda: simulation.delete_arrays(variable, period)) + + +@hypothesis.settings( + max_examples=300, + deadline=None, + suppress_health_check=[hypothesis.HealthCheck.too_slow], +) +@hypothesis.given(operations=_operations) +def test_fast_cache_never_changes_what_calculate_returns(operations): + cached = build_simulation() + uncached = build_simulation() + + for step, operation in enumerate(operations): + expected = _apply(uncached, operation, empty_fast_cache_first=True) + assert _apply(cached, operation) == expected, (step, operation) + + for variable, read_periods in READS: + for period in read_periods: + expected = _result(lambda: uncached.get_array(variable, period)) + assert _result(lambda: cached.get_array(variable, period)) == expected + + +_cached_period = st.sampled_from( + MONTHS + YEARS + ["2012-02-15", "month:2012-11:3", "year:2012:2", "2011"] +).map(periods.period) + + +@hypothesis.settings(max_examples=300, deadline=None) +@hypothesis.given( + cached=st.sets(_cached_period, max_size=8), + other_variable_cached=st.sets(_cached_period, max_size=3), + deleted=st.sampled_from(DELETE_PERIODS), +) +def test_delete_arrays_drops_from_the_fast_cache_what_it_drops_from_storage( + cached, other_variable_cached, deleted +): + simulation = build_simulation(carry_over=False) + holder = simulation.get_holder("monthly_formula") + for period in cached: + value = np.asarray([1.0]) + holder._memory_storage.put(value, period, "default") + simulation._fast_cache[("monthly_formula", period)] = value + for period in other_variable_cached: + simulation._fast_cache[("monthly_input", period)] = np.asarray([2.0]) + + simulation.delete_arrays("monthly_formula", deleted) + + in_fast_cache = { + period for name, period in simulation._fast_cache if name == "monthly_formula" + } + assert in_fast_cache == set(holder.get_known_periods()) + assert { + period for name, period in simulation._fast_cache if name == "monthly_input" + } == other_variable_cached diff --git a/tests/fixtures/fast_cache_contained_periods.py b/tests/fixtures/fast_cache_contained_periods.py new file mode 100644 index 000000000..33e263383 --- /dev/null +++ b/tests/fixtures/fast_cache_contained_periods.py @@ -0,0 +1,77 @@ +"""Variables and builders shared by the fast-cache containment tests.""" + +from __future__ import annotations + +from policyengine_core import periods +from policyengine_core.country_template import CountryTaxBenefitSystem, entities +from policyengine_core.holders import set_input_divide_by_period +from policyengine_core.simulations import SimulationBuilder +from policyengine_core.variables import Variable + + +class monthly_input(Variable): + value_type = float + entity = entities.Person + definition_period = periods.MONTH + label = "Monthly input" + + +class monthly_input_split(Variable): + value_type = float + entity = entities.Person + definition_period = periods.MONTH + label = "Monthly input an annual value is split over" + set_input = set_input_divide_by_period + + +class monthly_formula(Variable): + value_type = float + entity = entities.Person + definition_period = periods.MONTH + label = "Monthly formula of the monthly input" + + def formula(person, period, parameters): + return person("monthly_input", period) + 1 + + +class yearly_input(Variable): + value_type = float + entity = entities.Person + definition_period = periods.YEAR + label = "Yearly input" + + +class eternal_input(Variable): + value_type = float + entity = entities.Person + definition_period = periods.ETERNITY + label = "Eternal input" + + +class eternal_formula(Variable): + value_type = float + entity = entities.Person + definition_period = periods.ETERNITY + label = "Eternal formula of the eternal input" + + def formula(person, period, parameters): + return person("eternal_input", period) * 2 + + +VARIABLES = ( + monthly_input, + monthly_input_split, + monthly_formula, + yearly_input, + eternal_input, + eternal_formula, +) + + +def build_simulation(*, carry_over: bool = True): + """One person, with the variables above.""" + system = CountryTaxBenefitSystem() + system.auto_carry_over_input_variables = carry_over + simulation = SimulationBuilder().build_default_simulation(system, count=1) + system.add_variables(*VARIABLES) + return simulation