diff --git a/CHANGELOG.md b/CHANGELOG.md index c013acf3f..10d269bf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 41.5.0 [#1205](https://github.com/openfisca/openfisca-core/pull/1205) + +#### New feature + +- Add a look back constraint on spiral errors + ### 41.4.2 [#1203](https://github.com/openfisca/openfisca-core/pull/1203) #### Technical changes diff --git a/openfisca_core/simulations/simulation.py b/openfisca_core/simulations/simulation.py index 9e7e0034e..330bbe385 100644 --- a/openfisca_core/simulations/simulation.py +++ b/openfisca_core/simulations/simulation.py @@ -48,6 +48,8 @@ def __init__( # controls the spirals detection; check for performance impact if > 1 self.max_spiral_loops: int = 1 + self.max_spiral_lookback_months: int = 0 + self.memory_config = None self._data_storage_dir = None @@ -400,8 +402,10 @@ def _check_for_cycle(self, variable: str, period): raise errors.CycleError( "Circular definition detected on formula {}@{}".format(variable, period) ) - spiral = len(previous_periods) >= self.max_spiral_loops - if spiral: + + too_many_spirals = len(previous_periods) >= self.max_spiral_loops + too_backward = (previous_periods[0].date - period.date).in_months() > self.max_spiral_lookback_months if previous_periods and self.max_spiral_lookback_months > 0 else False + if too_many_spirals or too_backward: self.invalidate_spiral_variables(variable) message = "Quasicircular definition detected on formula {}@{} involving {}".format( variable, period, self.tracer.stack @@ -412,17 +416,9 @@ def invalidate_cache_entry(self, variable: str, period): self.invalidated_caches.add(Cache(variable, period)) def invalidate_spiral_variables(self, variable: str): - # Visit the stack, from the bottom (most recent) up; we know that we'll find - # the variable implicated in the spiral (max_spiral_loops+1) times; we keep the - # intermediate values computed (to avoid impacting performance) but we mark them - # for deletion from the cache once the calculation ends. - count = 0 - for frame in reversed(self.tracer.stack): + first_variable_occurrence = next(index for (index, frame) in enumerate(self.tracer.stack) if frame["name"] == variable) + for frame in self.tracer.stack[first_variable_occurrence:]: self.invalidate_cache_entry(str(frame["name"]), frame["period"]) - if frame["name"] == variable: - count += 1 - if count > self.max_spiral_loops: - break # ----- Methods to access stored values ----- # diff --git a/setup.py b/setup.py index b804cbb79..d4a2730bf 100644 --- a/setup.py +++ b/setup.py @@ -67,7 +67,7 @@ setup( name="OpenFisca-Core", - version="41.4.2", + version="41.5.0", author="OpenFisca Team", author_email="contact@openfisca.org", classifiers=[ diff --git a/tests/core/test_tracers.py b/tests/core/test_tracers.py index d0e25f866..76343d375 100644 --- a/tests/core/test_tracers.py +++ b/tests/core/test_tracers.py @@ -10,6 +10,7 @@ from openfisca_country_template.variables.housing import HousingOccupancyStatus from openfisca_core.simulations import CycleError, Simulation, SpiralError +from openfisca_core.periods import period from openfisca_core.tracers import ( FullTracer, SimpleTracer, @@ -28,13 +29,15 @@ class StubSimulation(Simulation): def __init__(self): self.exception = None self.max_spiral_loops = 1 + self.max_spiral_lookback_months = 0 + self.invalidated_cache_items = [] def _calculate(self, variable, period): if self.exception: raise self.exception def invalidate_cache_entry(self, variable, period): - pass + self.invalidated_cache_items.append((variable, period)) def purge_cache_of_invalid_values(self): pass @@ -120,12 +123,15 @@ def test_cycle_error(tracer): def test_spiral_error(tracer): simulation = StubSimulation() simulation.tracer = tracer - tracer.record_calculation_start("a", 2017) - tracer.record_calculation_start("a", 2016) - tracer.record_calculation_start("a", 2015) + tracer.record_calculation_start("a", period(2017)) + tracer.record_calculation_start("b", period(2016)) + tracer.record_calculation_start("a", period(2016)) with raises(SpiralError): - simulation._check_for_cycle("a", 2015) + simulation._check_for_cycle("a", period(2016)) + + assert len(simulation.invalidated_cache_items) == 3 + assert len(tracer.stack) == 3 def test_full_tracer_one_calculation(tracer):