Repository navigation
Conversation
…g over Holder.get_known_periods() lists the periods of every stored key, with the branch name stripped, but Holder.get_array() reads only the requested branch, its parent_branch ancestors and "default". Simulation._calculate took the latest known period from the unscoped list, so a period stored only under an unrelated branch read back as None: uprating raised TypeError and auto-carry-over cached NaN. - Holder._readable_branch_names() is the one definition of what a branch can read; get_array() and the new get_known_periods(branch_name) both use it. - _calculate uses get_known_periods(self.branch_name). - OnDiskStorage splits "<branch>_<period>" keys on the last "_": branch names like "no_salt" raised ValueError, "y_2019" listed the wrong period, and delete(None, "pre_tcja") also wiped "pre_tcja_ctc". - dump_simulation saves the values the dumped branch reads instead of reading every period under "default" and saving None. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 1, 2026
MaxGhenis
added a commit
that referenced
this pull request
Oct 2, 2026
…ut-only carry-over Round-2 review findings: - A default cached for a period with no earlier input became the base the uprating path uprated later periods from; the default is not cached for variables with uprating (as before). - A value calculated inside a set_input helper was written to the input's branch and recorded as a user input, bypassing the guard that keeps inputs; derived writes now stay on the branch they were calculated on. - dump_simulation reads each value's mark from the same branch and period as the value it dumps, so #552's branch dumps keep matching marks. - Carry-over checks candidates latest first and stops at the first input, instead of resolving the storing branch for every known period. - Storages drop the marks of deleted keys. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis
added a commit
that referenced
this pull request
Oct 2, 2026
… handlers From the round-5 soundness review (on 2b31dcb); all were also on master: - A branch stops reading macro-cache files as soon as an input is set on it, not only when the input drops something: a file for its name may come from another simulation's branch of the same name, read before any record exists. - An input set during a calculation for its own period wins under any branch name the simulation reads (Holder.set_input stores under "default"), and also when the formula returns None and core would fall back to the default for want of an earlier period. - A direct calculate_add sums again when a term changed an input while summing (an earlier term could be obsolete). - A custom set_input handler that calculated and then raised: the branch still drops again (the inputs it stored stay). The drop after a handler now runs only if the handler calculated anything. - calculate runs a calculation again until a run changes no input, at most ten times (was once), so a formula changing a second input on its rerun no longer leaves the period unknown to carry-over. - Docs: the above, and that disk-backed branches whose names contain "_" cannot be dumped (OnDiskStorage key parsing, as on master; #552 fixes it). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The string form of a multi-unit or rolling-year period contains ':', which a Windows file name cannot. On windows-latest, numpy.save raised OSError for month:2025-01:3 and year:2024:2, and year:2025-03 read back until restore() found no file for it. Disk storage has never kept these periods on Windows; this PR does not change that, so the test skips them there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
OnDiskStorage.delete(period, branch_name) removed only the file keyed
exactly f"{branch_name}_{period}". InMemoryStorage.delete and the
Holder.delete_arrays docstring remove every period the given period
contains, so a disk-backed holder kept values the caller had deleted
(deleting "2025" left "2025-01" and "2025-02"). Parse each key with
_split_key and delete those whose branch is branch_name and whose period
the deleted period contains. Eternal storage still deletes its one
ETERNITY key.
Fixes #564.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 2, 2026
MaxGhenis
added a commit
that referenced
this pull request
Oct 3, 2026
…562) * Carry over only inputs, the latest at or before the requested period Auto-carry-over took the latest-starting stored period of any kind and returned the default if it started after the requested period. Any later stored period therefore hid an earlier input: a later input (inputs for 2012 and 2014 gave the default for 2013), or a later period the simulation had already calculated (2014 calculated first made 2013 the default). And values the simulation calculated carried like inputs: a twelfth cached at a month by calculate_divide (the policyengine-us monthly_age bug #557 fixes), a value masked by defined_for, a default cached where defined_for was false everywhere, or a formula result from before the formula's end. Results depended on which periods were calculated first. The rule now: a period takes the input stored for the latest-starting period that starts no later than it (on a tie, the one ending last), preferring the variable's own definition-period unit; with none, the default. Values the simulation calculates are stored with derived=True: the mark lives in the storage with the value (InMemoryStorage and OnDiskStorage keep it per key, every put sets or clears it, it counts only while the key is stored, and clones copy it), so deletions, direct writes and #556's shared arrays cannot leave it stale. Holder.is_derived(period, branch_name) reports the mark of the value get_array reads, so a value one branch calculated never hides another branch's input. A derived value never replaces an input the branch reads (calculate_add/calculate_divide used to overwrite inputs stored in another unit), calculate_add caches only sums over several sub-periods, and dump/restore keeps the marks. Storages gain has(), which neither reads nor copies an array. Supersedes #557's carry-over change: its 253 tests pass here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep uprating, input helpers, dumps and deletions consistent with input-only carry-over Round-2 review findings: - A default cached for a period with no earlier input became the base the uprating path uprated later periods from; the default is not cached for variables with uprating (as before). - A value calculated inside a set_input helper was written to the input's branch and recorded as a user input, bypassing the guard that keeps inputs; derived writes now stay on the branch they were calculated on. - dump_simulation reads each value's mark from the same branch and period as the value it dumps, so #552's branch dumps keep matching marks. - Carry-over checks candidates latest first and stops at the first input, instead of resolving the storing branch for every known period. - Storages drop the marks of deleted keys. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test that deleting a whole branch drops its derived marks Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Cache the default only where master cached a carried value With no input at or before the period but a later period stored, master returned the default without caching it; caching it changed what formulas that test whether a value is stored see (policyengine-uk's maintenance loan and current_education formulas check get_array(period.last_year)), and gave the uprating path a calculated base. Return it uncached there, as master did; cache it (derived) otherwise, as master cached the value it carried. This replaces the uprating-only special case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Read input periods in one pass over readable keys; clear marks with the caches Round-3 review findings: - A period stored only under a branch this one cannot read was taken for an input (own-unit preference ranked it first) and read back as NaN. Holder.get_input_periods(branch_name) now lists, in one pass over the stored keys, the periods whose readable value is an input, taking for each period the key get_array reads first; carry-over picks from those. This also replaces the per-period walk up the branch chain. - apply_reform's cache wipe kept the storages' marks, and rebuilding a disk index brought a stale mark back onto a new input; the wipe now clears the marks and OnDiskStorage.restore starts without any. - Storages pickled before the marks existed failed to clone, put or delete; __setstate__ now defaults them (and #556's shared-key set). - The public carry-over rule now states the own-unit preference and that an input for the period itself is read back as stored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test memory-over-disk precedence, in-memory mark clearing and index rebuilds Closes three mutants the suite did not kill. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Break equal-extent carry-over ties by unit, then period Two inputs covering the same days in units other than the variable's (month:2013-01:2 and day:2013-01-01:59) resolved to whichever was stored first. Prefer the larger unit, then the period's string form, so the choice depends only on the inputs (round-4 review). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Record a storage's inputs, not its derived values, and share them with clones Measured on a policyengine-us household (policyengine-us 2.18.3, one household_net_income calculation, 6,185 storages, 30,925 holder clones across 5 branches), master with #578 retains 20.94 MB per simulation. With derived marks as a set per storage that holds a derived value, the merge retained 24.35 MB: 1,514 sets in the simulation and a copy of its source's marks in every holder clone. That is 2.6 times the 1.3 MB per simulation that ran policyengine-us's CI out of memory (#577). 5,201 of the 5,204 stored values are derived, so record the inputs instead: is_derived is "stored and not an input". The 3 storages holding inputs have a set (648 bytes); the rest read _NO_INPUTS. A storage and its clones share one frozenset of inputs until one of them changes its own. Retained memory is now 21.25 MB per simulation, +0.31 MB on master: one more 8-byte attribute slot in each of the 37,110 storage objects. A pickle records that inputs were recorded; a state from before (no marker) counts every stored value as an input, as it did then. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Carry over an input that ends after the last representable date The carry-over sort key evaluated period.stop for every candidate input. A period that ends after 9999-12-31 (day:9999-12-30:3, or day:2012-01-01:4000000) has no stop: computing it raises OverflowError. Master carried such inputs ([10, 20]); this branch raised. Sort by _end_order, which puts such a period after every period that has a stop, so it still ends last. Found by the review of #582, which uses the same key for uprating. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Order period ends by day number, so ends after 9999-12-31 compare too 6ddbaf9 sorted every period whose stop cannot be computed after those that have one, but equal to each other, so of two inputs ending after 9999-12-31 the string form decided: day:9999-12-30:9 was carried over day:9999-12-30:10, and 24 months over 800 days from the same day (delta review, P2). Month and year periods past 9999 also raised ValueError, or returned a stop no date can hold, rather than OverflowError. _end_order is now the number of the day after the period's last day, counted as date.toordinal counts, by integer arithmetic on the same calendar. It equals stop + 1 day wherever stop has a value and orders every other period by its true end. Tests: the review's two cases in both set orders; a property that _end_order agrees with Period.stop where it has a value and with numpy's calendar everywhere (1,000 examples, sizes to 5,000,000); a longer period from the same day ends later. Also asserts that a pickled state without a set of shared keys gets none (a surviving mutant). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 6, 2026
Reconcile #552 with master after the merge: - Holder.get_known_periods(branch_name) uses master's _readable_branches (added by #562) instead of a second copy of the lookup, and lists each period once. get_array is master's again. - _dump_holder reads the dumped simulation's branch from the holder, as #560 does, and keeps master's derived_periods.txt with each value's mark read on that branch. - The _calculate comment says what scoping still changes now that uprating and carry-over read only inputs (#562/#563): a later period stored under an unreadable branch no longer leaves a default uncached. Tests: the isolation grid now also compares what the branch then reads (periods and derived marks); a period stored under several readable branches is listed once; a restored branch dump calculates what the branch calculates; and a Hypothesis differential checks get_known_periods against get_array and get_input_periods against is_derived for random values on a five-branch lineage, in memory and on disk. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes the two findings of #565's review (REQUEST_CHANGES at bfa78cb): 1. An input for a month or year anchored mid-month (month:2025-03-15) was accepted on disk under the calendar month's key, since its string form drops the day; deleting its own period, which does not contain that calendar month, then left it. In-memory storage already rejected it. Both storages now run one check, storage_keys.check_storable, so whether an input is accepted no longer depends on which storage Holder._set picks by memory use; disk storage rejects branch names containing ':' too. 2. restore() read back every .npy file. A name that is not a key put writes (stray.npy, default_garbage.npy, default_2025-3.npy) made delete and get_known_periods raise, so a valid deletion left its value. restore now leaves such files out, with a warning naming them. Tests: examples on both backends and through Simulation for each finding; the memory/disk Hypothesis differential now draws mid-month periods and a ':' branch name and checks both storages accept or reject each put alike; a new Hypothesis property restores a directory of random keys and non-key names and checks restore reads back exactly the keys, warns about the rest, and then deletes like memory. Folds #565's changelog fragment into this PR's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cover the remaining #565 review witness, periods.period(999), on both string-key storage backends. Preserve supported twelve-month aliases and existing derived values after rejected writes. Extend the differential property across the early-year boundary and twelve-unit periods. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis
changed the title
MaxGhenis
added a commit
that referenced
this pull request
Oct 10, 2026
…eplaces (#560) * Drop what a branch input may have fed; add drop_computed_arrays A branch starts with every value its parent holds, and set_input on a branch used to keep everything calculated from the value it replaced, so a value the parent calculated before branching answered for the branch whatever its inputs said. Every stored array now carries a sequence number from one process-wide counter (store order), and storages record which values are inputs; both are copied with clones. A simulation family shares a StoreHistory of the first store of each variable and period (including values a holder does not keep) and of the first uprated or carried-over value of each variable. set_input on a branch drops the branch's non-input values stored at or after the earliest of those for an overlapping period; when there is none, nothing is dropped. Simulation.drop_computed_arrays() drops every non-input value, for branches whose policy changes; _invalidate_all_caches now uses the same input flags. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Start a new store history when subsample rebuilds the simulation Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Record values served from the macro cache or a spiral's default Neither is stored, but values calculated from them are, so set_input on a branch must see them in the store history. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Move the Hypothesis property into its own module that skips without Hypothesis The country-package smoke job installs no dev dependencies, so the module-level hypothesis import failed its collection. The synthetic system the examples and the property share moves to tests/fixtures/branch_input_invalidation.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep a store history per simulation; fix what two reviews found Each simulation now keeps its own store history: a branch copies its parent's, and a formula that calculates in another simulation takes in that simulation's history when calculate returns (fast-cache hits included). A drop prunes the records it made obsolete and records the kept inputs again. An unrelated store in a sibling branch, or in the other arm of a reform/baseline pair, no longer lowers a branch's threshold, so MTR-style and baseline branches calculate shared values once. Also from the soundness and code reviews: - a branch whose input dropped values stops reading macro-cache files (they are keyed by branch and period, not inputs); drop_computed_arrays too; - requires_computation_after counts a prerequisite requested before a drop; - values a custom set_input handler calculates are not inputs; - disk storage writes a new file per store, so a recalculation no longer changes what child branches or same-named branches read; - a value written into storage without a number counts as a dependency; - storages pickled before numbering still store (__setstate__); - derivative keeps inputs set after the simulation was built; - property test: an independent model of divided inputs, reused and forgotten branch names, a blacklist mode, derandomized CI examples with an environment soak; examples for each finding; - docs and docstrings say what the code does, with the remaining limits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep records while a formula runs; merge incrementally; survive pickling From the second soundness review (in progress): - A drop while a formula is still running in the simulation (its own set_input, or drop_computed_arrays) no longer prunes the history: the formula may hold values it read before the drop and store a result from them. Calculations in flight are counted per simulation; a clone starts at zero. - Merging is incremental: each history keeps a journal of its changes since it was created or pruned, so repeated calls into a simulation whose history keeps growing no longer re-read every record (quadratic before). - The merge map is keyed weakly, so temporary branches' entries go with them, and is left out of pickles (weak references do not pickle). - Unpickling a storage or history advances the process's sequence counter past the numbers it carries; disk restore keeps each key's most recently written file (numbers restart in every process). - Tests: drops inside a running formula, an uprated value handed back by a temporary branch (also in the property's scenarios), cross-process unpickling, restore order. Docs: the in-flight rule and the thread limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Hand branch history to waiting ancestors; tokened disk files; gate on success From the second soundness review: - A branch calculated from a thread with no formula context (a worker a formula starts without copying its context) hands its store history to every ancestor with a calculation running, so the waiting formula's result keeps the dependency. Unrelated simulations are not touched. - Disk files carry a per-process token as well as the sequence number: numbers restart in each process, so a later process could otherwise overwrite a file a restored view still maps. - requires_computation_after counts a prerequisite only once its calculation succeeded. - Docs and comments: the soundness argument states records for overlapping periods (summed and divided values), disk files stay until the directory is removed, the thread limit covers only unrelated simulations, and the history is copied into branches, not shared. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close the third review's gaps: in-flight results, failures, dumps, disk names From the round-3 soundness review: - A calculation still running when an input set on its simulation dropped values returns its result but does not cache it (an input epoch), so the next calculation uses the new input. - A calculation that raises still hands its store history to the calling formula: whether it fails can depend on what it read. - A merge records how far it read the source before reading, so a record added meanwhile (another thread) is read next time. - dump_simulation records each variable's input periods (inputs.txt); restore_simulation restores inputs as inputs, then every other value under one later number, so a branch input drops what it may have fed. Older dumps restore every value as an input, as before. - Disk restore reads the previous `<key>.<number>.npy` names, prefers this process's file on a timestamp tie, and a forked child gets its own token. - StoreHistory pickles from before journals load with empty journals. - Tests for each; docs updated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test pickling with a live weak read position; tidy disk storage imports The pickle test merged a history that died at once, so the weak map was empty when pickled. Keep the source alive so the test covers the case the __getstate__ exists for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fork the token test's child directly instead of through a process pool A process pool forked from a multi-threaded process (the test process can have threads by then) can deadlock; the child now only writes its token to a pipe and exits. Still fails without the at-fork hook. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close the fourth review's gaps: refused results, dumps, handlers, macro reads From the round-4 soundness review (on bff21a7): - A result refused because an input changed while it was calculated left its period unknown, so later carry-over found nothing (0 instead of 3). calculate now runs such a calculation once more from the new inputs and keeps that result; a second change still returns without keeping. - An input a formula sets for the very period it calculates is the result and is no longer overwritten by the formula's return value. - A dump does not say what each value was calculated from, nor what was read without being kept, and a macro-cache value's sources were never calculated here: the store history now records the number from which values may depend on anything (restored values, the first macro-cache read), so an input for any variable on a branch drops them. - A custom set_input handler that calculates between its own stores: the branch drops again, by the same rule, once the handler returns. - dump_simulation on a branch dumps the values the branch reads (its own and inherited) instead of the default branch's, which could not be restored. - calculate ends the calculation (in-flight count, tracer) even if handing its history back fails. - The property test also dumps and restores simulations, including a chunk that calculates, dumps, branches the restored simulation and overrides an input; it catches a history without the new record. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * DIAG: report traced parameter trees on Windows (to be reverted) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Give the disk-restore test's later process its own token, and age all earlier files It faked a later process by restarting the counter only, so with the same token its first store could reuse the writer's first file name; and it aged only the writer's last file, so on a coarse file clock (Windows) the other four could tie with the later write. Passes with every write given the same timestamp. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * DIAG: write the diagnostic through the terminal reporter (to be reverted) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * DIAG: tolerate half-built parameter nodes (to be reverted) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Revert the Windows diagnostics (3075a6c, a81adb4, 1d6221f) They found the cause: test_disk_restore_reads_the_latest_file_of_each_key failed on Windows' coarse file clock (fixed in acc5f58), and its rerun by pytest-rerunfailures dropped the module's setup-state entry without running its finalizers, so the module-scoped tax_benefit_system leaked into later modules; test_branch_shared_arrays traced it, and test_parameters and test_reforms then got a traced parameter tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close the fifth review's gaps: macro reads, input wins, sums, failing handlers From the round-5 soundness review (on 2b31dcb); all were also on master: - A branch stops reading macro-cache files as soon as an input is set on it, not only when the input drops something: a file for its name may come from another simulation's branch of the same name, read before any record exists. - An input set during a calculation for its own period wins under any branch name the simulation reads (Holder.set_input stores under "default"), and also when the formula returns None and core would fall back to the default for want of an earlier period. - A direct calculate_add sums again when a term changed an input while summing (an earlier term could be obsolete). - A custom set_input handler that calculated and then raised: the branch still drops again (the inputs it stored stay). The drop after a handler now runs only if the handler calculated anything. - calculate runs a calculation again until a run changes no input, at most ten times (was once), so a formula changing a second input on its rerun no longer leaves the period unknown to carry-over. - Docs: the above, and that disk-backed branches whose names contain "_" cannot be dumped (OnDiskStorage key parsing, as on master; #552 fixes it). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close the sixth review's gaps: input wins only if stored meanwhile, one rerun budget From the round-6 soundness review (on 9f8d513): - Direct calculate_divide/calculate_add could return an input stored before the calculation (they do not look in storage first), after any unrelated input change: 99 instead of 10, 100 instead of 12. An input now wins only if its sequence number is after the calculation began. - Only the outermost calculation in a simulation (calculate or a direct calculate_add) runs again after an input change; inner ones run with it. The ten-rerun budget no longer multiplies through nesting (11/121/1,331 leaf runs at depths 1/2/3, now 11 each). - A result calculated across an input change is no longer written to the macro cache (only kept results are), so a branch of the same name elsewhere cannot read it (also on master). - The post-handler drop counts _calculate, not only calculate, as calculating (a handler's private carry-over calculation was missed). - Docs: the above, and the rerun cap's consequence for carry-over. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close the seventh review's gaps: refused results stay unkept across simulations From the round-7 soundness review (on ed843df): - A parent formula calling into a branch whose formula calls back into the parent and then changes the branch's input: the parent kept the branch's refused inner result (2 instead of 6; also through calculate_add and calculate_divide). Drops made while calculations run now also bump a process-wide count, and no result calculated across one, in any simulation, is kept. Which calculation runs again is still decided per simulation: the outermost one in the simulation whose input changed, so a temporary branch settles locally and nesting does not multiply reruns. - A direct calculate_add counts as in flight while it sums, so its terms do not each get their own reruns (121 runs, now 11). - Inputs are numbered when stored, even if the caller passes a number (a private _set with an earlier number lost its input to "input wins"). - Tests for each; docs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close the eighth review's regression: scope "not kept" to callers and family From the round-8 soundness review (on 116b018): refusing to cache any result calculated across an in-flight drop anywhere in the process let an unrelated simulation's input change leave a period uncached, so a later carry-over returned 0 instead of 6 (master, 9f8d513 and ed843df give 6). A result calculated across an input change is now kept out of caches only where it can matter: - in the simulation whose input changed (its own epoch, as before); - in any calculation that received a refused result, directly or through other calculations, in any simulation (a per-frame flag in _calculation_frames, passed to the caller when a frame returns); - in calculations of the same family still running in this context, which may hold values read before the drop (their cache epoch is bumped). Unrelated simulations and other threads keep caching. Reruns stay with the outermost calculation in the simulation whose input changed; calculate_add and calculate_divide run in frames too. Docs: the scope, and that values a formula already read from a branch before a formula there changed the branch's input stay read (its result is returned but not kept). Tests isolate each part (taint across families, the family bump, the unrelated family); new mutants M38/M39 are caught. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close the ninth review's gaps: refuse by what each calculation read From the round-9 soundness review (on 359c97f): - A calculation that changed its simulation's input and then left before any cache decision (a carry-over default returned early, an exception) did not tell its caller, which kept an obsolete result (0 instead of 6). - A formula that read another simulation (another family, a root clone, its own branch from a plain thread) and then calculated there something that changed that simulation's input kept its old result (2 on the second request instead of 6). - The family bump refused a settled, correct result, so a later carry-over found no period (0 instead of 6; master gives 6). One rule replaces the taint flag and the family bump: each calculation frame records, for every other simulation it got a value from (directly or through its callees), that simulation's _input_epoch when the value's calculation began; it keeps its result only if neither its own simulation's epoch nor any of those has changed. A callee hands its reads to its caller however it ends (value, early default, error), its own simulation counted at its start; a rerun that settles restarts the count, so a settled call leaves its caller clean. Fast-cache hits from another simulation are reads too. Docs updated ("values already read stay read" now covers any simulation and caught errors). Tests for each witness; mutants M39 (no reads) and M41 (a read counted from return, not start) are caught. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Close the tenth review's gaps: thread reads, settled reads, idle changes 1. A value read in a thread started without the formula's context now counts as a read: with no calling frame, a calculation's reads go to every frame open in its simulation's ancestors (each simulation lists its open frames). Before, a parent that got its branch's value that way kept it after the branch's input changed. 2. The outermost calculation in a simulation now runs again while its frame is stale, not only while its own input epoch changed, so a formula that read a value before another simulation's input changed gets the current one and keeps it. Before, a correct settled result went unkept and an earlier read stayed in the returned value. 3. Every drop counts as an input change of the simulation, also when nothing is running there, so a formula that read a simulation and then set an input on it directly does not keep what it read. 4. Setting a branch input to the value the branch already reads there, as an input, drops nothing (not for a value calculated there, nor through a set_input helper). Without this, a formula that sets the same input on every run would rerun until the budget ran out. The property test now also sets inputs again with the value read. Docs describe the new rules, and the limit that a value kept in one simulation stays when another's input changes after its calculation ended. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Remove synthetic_simulation's temporary folders when done (US + core hub patch) Each disk-backed synthetic simulation gets a folder inside one folder per test process, removed when the simulation is collected or at exit, and its pre-created disk storages leave removal to that, so a killed run leaves one folder, not one per live simulation. Patch from the US + core hub (policyengine-core#585 notes). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Bring two docstrings in line with the merged rules StoreHistory: uprating and carry-over read which periods hold inputs. Simulation.set_input: drops count as input changes, stale calculations anywhere run again, unchanged inputs drop nothing, and values kept elsewhere stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep macro reads off on unchanged inputs; cut read tracking's per-call cost Review of 789d7f0 (round 11, read-only): - The unchanged-input short-circuit also skipped the branch's macro-cache switch-off, so a branch given the input it already read kept reading files another simulation's same-named branch may have written. Holder. set_input now turns macro reads off on every branch input. - -0.0 no longer counts as the input 0.0 (1 / x tells them apart). - The changelog states the short-circuit's conditions. Cost: a single policyengine-us household (household_net_income, 2026) took about 20% longer than on master, all in per-calculation bookkeeping. Now: - a calculation nested in another in the same simulation shares that one's frame (it began no later and notes at least the same reads, each at its first epoch, so sharing only keeps less); a frame opens only at the outermost calculation in a simulation or on entering another one; - frames are a slotted class, their own context manager and registry key; - counters are class attributes, read without getattr defaults. The household now takes about 4-5% longer than on master. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test a helper's input over a stored year, and a callback that changes its input Two mutants of the round-10 mechanisms survived every test: an equal input given to a set_input helper skipping the drop, and a call into another simulation sharing its caller's frame. Each test fails under its mutant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Stop macro reads in one place: Holder.set_input on every branch input _drop_values_that_may_depend_on switched them off too, but it only runs from Holder.set_input after that has; the mutation check could not tell the second switch-off from none. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep copied disk storage invalidation order independent * Settle repeated branch input transitions without caching stale reads * Keep fixed-point readers and saved branch snapshots distinct * Retain registered branch paths across calculation retries * Guard fixed-point writes to optional fast caches * Keep independent calculations cached during stale branch retries * Cover shared credit dependencies during stale retries * Isolate early-year branch replay regression from fallback parsing * Keep raw cache writes observable during guarded retries * Keep branch invalidation changelog concise * Clarify root simulation helper cache cleanup * Forward store sequence numbers in restore property observer * Reuse stable foreign reads within stale calculation attempts * Capture branch registrations before memo-producing formulas run * Guard attempt reuse with the branch name actually resolved --------- 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
1 participant
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.
Follow-up to #551; folds the contained-period disk deletion fix and tests from #565 (fixing #564). #565's branch is preserved and neither PR has been closed.
Problem and change
Unscoped known-period listings let values stored only on unreadable branches affect a simulation's fallback caching. Disk keys split on every underscore misread branch names such as
no_salt, and prefix deletion also removed other branches. Dumps read default-branch values instead of the dumped branch's values. Disk deletion removed only an exact key, despite the holder's documented containment rule.Holder.get_known_periods(branch_name)now deduplicates periods visible through master's existing ancestor lookup. Simulation fallback uses that scoped listing. Dumps save the visible value and its matching derived mark using the existingderived_periods.txtformat. Disk keys split on the last underscore, whole-branch deletion matches the complete branch name, and period deletion removes contained periods while retaining other branches and derived marks for surviving values.The branch includes a history-preserving merge of canonical master
5d68130be487f0b87d47fc75ef300f36ad46f7ca. It preserves master's input-only uprating/carry-over, input/derived bookkeeping, and #585'sTemporaryStorageDirectory, ownership, clone/fork handling and_path_to_writearchitecture. The dumper conflict retains current derived marks; no parallel marks file is introduced.Folded review findings
Simulation.set_input("rent", "month:2025-03-15", [500])previously survived deletion on disk because its key lost the start day. Both backends now apply the existing memory restrictions before writing.stray.npyordefault_garbage.npypreviously poisoned unrelated valid deletion. Restoration warns and skips filenames without a branch separator followed by a canonical period string.put(..., periods.period(999)), whose filename cannot be reparsed. Both string-key backends now reject unparseable period strings before changing stored values. Supported twelve-month/one-year aliases remain accepted.The original #565 commit
bfa78cba8f229f9029dd17fe6609e8f91644c2a3descends from #552's previous head5fe8b61864a3a929d3394e1658a83de1f4ccf7beand is an ancestor of this head. Its examples and Hypothesis differential are retained and extended, and its changelog is folded into the single #552 fragment. Its draft status and branch remain untouched; merging the old #565 branch separately would duplicate this fold.Invariants and coverage
Validation at 0432ced
Passed: Ruff 0.16.7 formatting and lint checks for all ten changed Python files,
git diff --check, ancestry checks, and patch-bundle verification.Fresh independent review of this exact complete source head: APPROVE, no unresolved P1/P2. The reviewer separately checked formatting, lint, ancestry, documentation and the complete master-to-head diff. Runtime verification remains blocked as below.
Focused pytest commands were attempted through
~/reviews/us-hub/scripts/heavy_run.sh; the wrapper exits 75 because this sandbox denies itspscalls. Pytest never starts. No fresh test counts, fail-before/pass-after execution, or downstream impact results are claimed. Historical counts from the previous PR description do not validate this head.Required follow-up: run the four branch visibility/disk deletion test modules plus relevant holder, dump/restore, anchored-period, provenance, uprating/carry-over and #585 clone regressions through the hub wrapper. Full suites, country-template CLI, documentation build, Windows execution and microsimulations were not run in this bounded build. CI must validate the new head.
Downstream impact
Re-run pending at
0432ced3067eafa12f02e4b13b3cab1e5186c367. The hub's existing US/UK A/B and partner-impact gates remain for Max; this build performs no new microsimulation and edits no partner baseline tests.Holds and composition
Core-semantics merge remains reserved for Max; no separate affirmative decision or decision ID was found. This PR is not declared ready. Independent current-head source review is complete; current green CI and the remaining hub gates are still required.
#560 and the other storage/restore stack changes remain separate work. Whichever branch lands later must preserve the current derived marks and #585 storage architecture. This repair does not adopt #560's invalidation semantics or a new dump format, or establish #576's separate restored-input registration contract.
axiom: n/a: core engine storage and branch lookup repair; no policy rule changes.
🤖 Generated with Claude Code