Repository navigation
Conversation
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 7950c01 and b78b0ba. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 2, 2026
Draft
MaxGhenis
added a commit
that referenced
this pull request
Oct 7, 2026
…ths (#576) Copy the three #566 fixture/test files unchanged from c825cab without its production change or fragment. Add disk clone/branch and shared-key cache regressions plus an on-disk cache/reference differential property, preserving master provenance and eviction. Keep #566 open and its branch unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis
added a commit
that referenced
this pull request
Oct 8, 2026
…ct non-numeric uprating and defined_for (#576) * Restore inputs as inputs, evict the fast cache on holder writes, reject non-numeric uprating and defined_for Three bugs found by the review of #563, all present on master b78b0ba: - restore_simulation put every value back with put_in_cache, which records no input, so apply_reform on a restored simulation dropped its inputs. dump_simulation now lists each variable's input periods in inputs.txt and restore_simulation records exactly those in _user_input_keys. - Holder.set_input, put_in_cache and delete_arrays left the simulation's fast cache untouched, so calculate kept returning the replaced value. Every holder write and delete now drops the entries it changes, in the holder's own simulation and only for branches that simulation reads. - An Enum, str or date variable with uprating, and a variable defined_for one, raised TypeError in the middle of a calculation. Both are now rejected when the variables are registered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Make the legacy-dump test strip every sidecar file, not only inputs.txt A dump written before inputs were recorded holds the arrays and nothing else, so the test stays right when another change adds its own sidecar. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Define the fast-cache eviction helper at the end of Holder Next to delete_arrays, git merged it with other changes to that method without a conflict but left their lines inside the new helper. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep the branch fast-cache tests valid if a branch input also drops what was calculated from it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review: branch inputs in dumps, replace rollback, calculate-time checks, legacy-dump warning - Test that an input set on a branch (which shares the input record) does not mark the default branch's calculated value as an input, with a branch step in the restore property. - replace_variable keeps the existing variable when the replacement is rejected. - calculate repeats the uprating check before it uprates, for an uprating assigned on a class that declares uprating itself; the defined_for message is tested for str and date as well as Enum. - The message for an inherited uprating points to replace_variable. - restore_simulation warns when a dump does not record its inputs. - Changelog: say that such systems no longer load, including a group variable defined_for a person Enum, which master masked on summed indices. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test the branch-input case by its outcome, not by whether the record is shared policyengine-core#561 makes a branch copy the input record instead of sharing it; the test now holds either way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 2: check defined_for by type before mapping; pin three cache paths - calculate checks the defined_for variable's type before calculating and mapping it. Mapped to a group entity, an Enum's indices were summed into numbers and str or date values failed inside the mapping, so the values-based check missed a cross-entity defined_for set after registration. - Regression tests for three cache paths no test pinned (each one a surviving mutant in GPT-6.1 Sol's review): a write under an ancestor branch name in a nested branch, a write to disk storage, and an ETERNITY value requested without a period. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Use derived-period dump provenance and rebuild input replay records (#576) Write a format marker so modern all-input dumps can be distinguished from ambiguous legacy dumps. Register non-derived restored values through the input context without dispatching helpers again; preserve calculated marks. Cover cached storage inputs, stale input records, Enum replay and differential round trips, and remove the obsolete calculated-month helper exception now fixed on master. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fold #566 regression tests and cover current-master holder storage paths (#576) Copy the three #566 fixture/test files unchanged from c825cab without its production change or fragment. Add disk clone/branch and shared-key cache regressions plus an on-disk cache/reference differential property, preserving master provenance and eviction. Keep #566 open and its branch unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Describe master input-only uprating in the variable API (#576) Correct the inherited attribute documentation to use the latest visible earlier input rather than a calculated or defaulted cache entry. No runtime change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Holder.delete_arrays(period)deletes every stored period thatperiodcontains. Deleting"2012"also deletes"2012-01"to"2012-12", and any period deletes an ETERNITY variable's one value.set_inputhandlers likewise store every month of an annual input.The simulation's
_fast_cache(checked at the top ofcalculate) was evicted by popping only the exact(variable, period)key. Socalculatekept returning values storage had deleted or replaced.Found by the adversarial review (GPT-6.1 Sol, 2026-10-01) of #562. It reproduces on
master7950c01 and b78b0ba. Each case below returns the stale value onmasterand the right one here:masterdelete_arraysset_input("m","2012-01",[100]),calculate("m","2012-02"),delete_arrays("m","2012"),set_input("m","2012-01",[200]).get_array("m","2012-02")isNone.calculate("m","2012-02")is 100delete_arrays2 × e: calculate for 2013,delete_arraysit for 2012, seteto 7set_inputset_input(..., "2013", [5]). Storage holds 5.set_inputset_input_divide_by_period: February calculated (0), thenset_input(..., "2012", [1200]). Storage holds 100.purge_cache_of_invalid_valuesm + 1calculated for 2012-02, then(name, "2012")invalidated and purged, thenmset to 20Fix
Simulation._evict_fast_cache(variable_name, period=None)drops each of the variable's fast-cache entries whose periodperiodcontains, using the samePeriod.containstest asInMemoryStorage.delete. It drops all of the variable's entries whenperiodisNoneor the variable is defined for ETERNITY.delete_arrays,set_inputandpurge_cache_of_invalid_valuescall it instead of popping one key.clone, and soget_branch, start a new one), so the scope matchesdelete_arrays: this simulation, not its branches.delete_arrayswithout a period already rebuilt the whole dict.Invariants (Hypothesis properties in
test_fast_cache_contained_periods_property.py)calculatereturns. Random sequences ofset_input,calculateanddelete_arraysrun on two simulations; one empties its fast cache before everycalculate. Every result agrees (value, or error type), and so does every stored value at the end.delete_arraysdrops from the fast cache exactly what it drops from storage. Afterwards the variable's fast-cache periods equal its stored periods, for any mix of day, month, year and multi-unit periods. Other variables' entries are untouched, so nothing is evicted needlessly.Tests
tests/core/test_fast_cache_contained_periods.py: 7 example tests.masterb78b0ba.tests/core/test_fast_cache_contained_periods_property.py: the two properties.master. Shrunk example: calculate a monthly input for 2012-01 (default 0),delete_arraysit for ETERNITY, calculate again.masteranswers from the stale entry and never stores the value again.pytest.importorskip("hypothesis"), because the smoke job installs no dev dependencies.tests/fixtures/fast_cache_contained_periods.py.test_fast_cache.pyandtest_fast_cache_guards.py): all 7 mutants killed.uvx ruff format --check .anduvx ruff check .pass.masterb78b0ba vs this branch, same country code and data): outputs are bitwise identical, so no published number moves.marginal_tax_rate; user CPU 9.49 s vs 9.45 s.marginal_tax_rate, which creates a branch per adult and callsdelete_arraysandset_inputon it): 25 of 25 arrays identical; income tax $1,962.456bn on both.Not in this PR
derivative()overwriting the source's input (the same review's other finding).OnDiskStorage.cloneshares.npyfiles, so the clone'sset_inputoverwrote the parent's file: derivative[0], input afterwards[101].[1], input stays[100], carry-over 100) and is a regression test there (test_derivative_of_a_carried_over_input_leaves_it).OnDiskStorage.delete(period)removing only the exact key (OnDiskStorage.delete(period) deletes only that exact period, not the periods within it #564). With disk storage, deleting"2012"leaves"2012-01"and"2012-02"in storage, so the first table row still returns 100 from disk. Delete the periods within a deleted period from disk storage #565 fixes that.Holder.delete_arraysandHolder.set_inputcalled directly (code that bypassesSimulation) still leave the simulation's fast cache alone, as before. Direct callers on current mains (PE-US fbe24ad1b3, PE-UK c7e826ea5):system.py(input renames) and PE-UKSimulation.move_valuesrun while the simulation is built, before anything is calculated.tob_revenue_oasdi.pyandtob_revenue_medicare_hi.pycallholder.set_inputon a branch created a moment before, whose fast cache is still empty.tools/branched_simulation.pyis imported but not used.utils/solve_private_school_attendance_factor.pydeletes on a freshMicrosimulation's baseline before calculating.utils/dependencies.py(an analysis helper, not part of model runs) deletes through the holder right aftercalculate, so a latercalculatewithoutmap_toreads the old value from the fast cache. That is a bug in the helper, to fix there withsim.delete_arrays; this PR doesn't change it.Overlap with other PRs
Simulation.delete_arraysdocstring above the:paramline this PR edits. They are separate hunks.drop_computed_arraysright afterdelete_arrays, and its store-history helpers right afterset_input, next to the eviction lines this PR replaces. Whichever lands second keeps both sides of those two hunks. Make set_input on a branch drop values calculated from the input it replaces #560 also changes when_calculatewrites into_fast_cache, which this PR doesn't touch.fix-stock-option-caches) leaves the eviction line inpurge_cache_of_invalid_valuesto this PR. It may change the holder-deletion branch scope on the line before.axiom: n/a: core cache infrastructure, no policy change
🤖 Generated with Claude Code