Skip to content

Carry over only inputs, the latest at or before the requested period - #562

Merged
MaxGhenis merged 11 commits into
masterfrom
fix-carry-over-order
Oct 3, 2026
Merged

MaxGhenis merged 11 commits into
masterfrom
fix-carry-over-order

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

With auto_carry_over_input_variables = True (policyengine-us, -uk, -canada), a variable with no formula result for a period takes a value from another period. Simulation._calculate took the latest-starting stored period of any kind and returned the default if that period started after the requested one. So the result depended on which periods had been calculated first:

  • Any later stored period blocked carry-over. Inputs for 2012 and 2014 gave the default for 2013. With only a 2012 input, calculating 2014 first (which carries 2012 and caches 2014) made 2013 the default too.
  • Calculated values carried like inputs. A month's twelfth cached by calculate_divide carried into later years (the policyengine-us monthly_age bug Carry over only values stored at the variable's definition period #557 fixes). So did a value masked by defined_for (its mask came along), a default cached where defined_for was false everywhere (it replaced the input), and a formula result from before the formula's end.

Executed on master b78b0ba vs this branch. Each row compares a simulation that calculated something first with a fresh one asked only the target (one-person YEAR variables; script explore.py in the review folder):

Scenario Fresh Master after earlier request This PR
Input 2012 = 7; ask 2014, then 2013 7 0 7
Inputs 2012 = 7 and 2014 = 9; ask 2013 0 on master, 7 here 0 7
defined_for false for person 0 in 2013 only; ask 2013, then 2014 [7, 7] [0, 7] [7, 7]
defined_for false for everyone in 2013; ask 2013, then 2014 7 0 7
Formula ends 2013; ask 2013, then 2014 0 6 0
Yearly flow input 12 for 2012; ask 2012-06, then 2013 (#557) 12 1 12
Branch sets the 2012 input to 70 after the parent carried 2013; branch asks 2014 70 7 70
No input; ask 2013; then set 2012 = 7 and ask 2014 7 0 7

A policyengine-uk household from another session shows the same thing on PE-UK main: with region set for 2025, asking 2040 and then 2026 gives LONDON (the default) for 2026; asking 2026 first gives SCOTLAND. On this branch both orders give SCOTLAND.

Intended rule (from history)

No country package relies on "a later stored period means default" in its tests: the PE-US and PE-UK YAML suites were run with a shadow core that logs every carry-over decision where this rule and master's differ (results below).

Rule now

A period with no formula result takes the input stored for the latest-starting period that starts no later than it (on a tie, the one that ends last, then the larger unit), preferring the variable's own definition-period unit and using another unit only when there is none (as #557 does, and as before 3.24.0); masked by defined_for at the period; with no such input, the default. An input never carries backwards. Values the simulation calculated never carry. This is documented on TaxBenefitSystem.auto_carry_over_input_variables.

Fix

  • Provenance, stored with the value. Every value Simulation caches (formula, carried, uprated and default values; defined_for defaults; calculate_divide's month twelfth; calculate_add's multi-period sums) is stored with derived=True. InMemoryStorage and OnDiskStorage keep the mark per storage key: every put sets or clears it, delete drops it, clone copies it (including Share cached arrays with simulation branches and copy them on first read #556's shared-array clones), apply_reform's cache wipe clears it, OnDiskStorage.restore starts without it, and storages pickled before it existed default to none (__setstate__). A value calculated while an input is being set (by a set_input helper that calculates) stays on its own branch and is not recorded as an input.
  • Which periods are inputs. Holder.get_input_periods(branch_name) makes one pass over the stored keys and, for each period, takes the key get_array(period, branch_name) reads first (the branch, its parent_branch ancestors, default; memory before disk), keeping it if it is not derived. A period stored only under a branch this one cannot read is never taken for an input, and no per-period branch walk is needed. Holder.is_derived(period, branch_name) answers the same question for one period; both read no array, so branches still copy only what they read (Share cached arrays with simulation branches and copy them on first read #556).
  • Carry-over takes, of those input periods starting no later than the requested one, the latest: own unit first, then by start, then by end, then the larger unit, then the period's string form. The choice depends only on the inputs, not on the order they were stored. With none, the default; it is cached (derived) except when a later period is stored, where it is returned uncached exactly as master did, so formulas that test whether a value is stored (get_array(period.last_year) is not None) see what they saw before and the uprating path never uprates from a cached default.
  • Inputs are never replaced by derived values. put_in_cache(..., derived=True) stores nothing when the branch already reads an input at that period. calculate_add/calculate_divide used to overwrite inputs stored in another unit (helper-less variables) or over several years; calculate_add now caches only sums over several sub-periods (a single sub-period is already stored by calculate).
  • Dump/restore persists the marks (derived_periods.txt per variable), read in the same loop and on the same branch as the values dumped (so Read only periods the current branch can see when uprating or carrying over #552's branch dumps, once merged, keep matching marks: the merge conflicts there on purpose).

Relation to open PRs

Invariants

For every input set and every sequence of earlier requests (calculate in either unit, calculate_add, calculate_divide, on the simulation and on branches forked from it):

  1. Order independence. The value a simulation (or a branch or nested branch) returns for a period equals what a fresh simulation given the same inputs returns for that period alone. Bitwise.
  2. Reference rule. It equals the rule above, computed directly from the inputs (tests/fixtures/carry_over.py::reference).
  3. No backward carry. With inputs only after the period, the default.
  4. Inputs survive options. After calculate_add/calculate_divide, every input a branch reads is still stored and still carries.
  5. Unchanged where master already agreed. Single-year PE-US and PE-UK outputs are bitwise identical to master (measured below).

Outside this PR's invariant (pre-existing on master, filed separately): uprating (#563, stacked on this PR); ADD/DIVIDE results read back by STOCK variables; the spiral depth cutoff; subsample exporting calculated values as inputs; on-disk derivative overwriting the parent's input file; _fast_cache eviction on period containment; deleted inputs left in _user_input_keys; on-disk keys with _ in the branch name (#552).

Tests

  • tests/core/test_carry_over_order.py: 56 regressions, no Hypothesis import; on master b78b0ba most fail, and those that pass are controls for behaviour that must not change (no backward carry, ADD over an input period, an off-unit input starting with the period, the year-first tie, reform replay of an ancestor input, the copy-on-write read count, defaults as master cached them).
  • tests/core/test_carry_over_order_property.py: the Hypothesis property for invariants 1 and 2, 400 derandomized examples plus an @example per regression shape, with branch-side requests and variables with no set_input helper. Skipped where Hypothesis is not installed (the smoke job).
  • tests/fixtures/carry_over.py: shared variables and the reference rule.
  • Mutation check (mutation_check10.py plus targeted reruns, 33 mutants of the carry-over rule, get_input_periods, the derived marks in holder and both storages, the input guard, ADD/DIVIDE, default caching, the cache wipe, index rebuilds, old pickles and dump/restore): all 31 non-equivalent mutants killed, 12 by the property alone (the rest pin branch, storage, dump and caching paths outside its domain). The 2 survivors are equivalent for current callers: caching a single sub-period's ADD result as derived, and has() implemented through get.
  • Randomized soak: 20,000 non-derandomized examples passed on the final commit.
  • uv run pytest tests: 1200 passed, 4 skipped, 1 xfailed (local, Python 3.14t). CI runs on Ubuntu and Windows, Python 3.11-3.14, and the smoke job.
  • Independent reviews (all executed reproductions): round 1, soundness (GPT-6.1 Sol) and code review (Opus 5.5); rounds 2 and 3, soundness (GPT-6.1 Sol). Every finding in this change's scope is fixed with a regression test that fails without the fix: ADD/DIVIDE overwriting inputs (multi-year, other-unit, anchored, in branches), per-branch provenance, provenance after deletion, uncached defaults, a cached default becoming an uprating base, ties, dump/restore and its branch, the input-helper branch context, copies of Share cached arrays with simulation branches and copy them on first read #556's shared arrays, the per-decision scan cost, the smoke-job import, the property's weakness, the changelog length, unreadable branches, marks across the cache wipe, index rebuilds and old pickles. Pre-existing issues outside this change are filed separately (listed under Invariants). Round 4 (GPT-6.1 Sol, on the round-3 fixes): APPROVE, with two non-blocking notes. First, a P3 performance tradeoff: _calculate decodes stored keys twice per decision (get_known_periods and get_input_periods), which costs extra only for histories beyond the period parser's cache (about 1,024 periods); left as a follow-up. Second, an insertion-order tie between equal-extent inputs, now broken by unit.

Measured impact

Every number below is from a real run: policyengine-us 6c5170fd and policyengine-uk 7b9fc379 (both main on 2026-10-01), core master b78b0ba versus this PR (35e20d9; later commits are tests only). Scripts, logs and arrays are in the review folder (us_bench.py, uk_bench.py, compare.py, full/, round5/).

PE-US, full Enhanced CPS, 2026, single year: 23 of 24 benchmark arrays bitwise identical: income tax $2,141.0853bn, state income tax $521.6485bn and household net income $14,462.0966bn in both, and every other tax, credit, SNAP and household array. One differs, spm_unit_net_income, for 14 SPM units (6,223 weighted), +$3.01m. That is a fix:

  • aca_magi_fraction reads the prior-year guideline, tax_unit_fpg for 2025, which reads state_fips for 2025. The dataset stores state_fips for 2024, and the run caches 2026 first. On master, the cached 2026 blocks carry-over to 2025, so every household gets the default state_fips 6 (California) for 2025.
  • With California's (contiguous) guideline instead of their own, Alaska and Hawaii households had too low a guideline. With this PR, tax_unit_fpg for 2025 changes for 320 SPM units, aca_magi_fraction for 292, and aca_ptc for 21 tax units: +$3.38m, from $40.8246bn to $40.8280bn (7,130 weighted). Medical out-of-pocket spending net of the credit falls $3.01m for 14 SPM units, raising their SPM net income by the same amount. aca_ptc enters household health benefits, not household net income.
  • A shadow core that logs every carry-over decision where this rule and master's differ recorded 3,474 decisions on the full run, 2,675 with a different source. Only this one changed a value; the rest returned the same value (master carrying a calculated default that equals the default).

PE-US, 3,000-household subsample, 2026: 25 of 25 arrays bitwise identical (including marginal tax rates and the itemization branches).

PE-UK, full Enhanced FRS 2024-25, 2026: 36 of 36 arrays bitwise identical (household net income £1,757.970bn, Universal Credit £78.494bn, income tax £313.139bn). The shadow log recorded 6 carry-over decisions, 3 with a different source and none with a different value.

YAML suites under the shadow core:

  • PE-UK: 1,370 passed; 0 decisions with a different value.
  • PE-US baseline: two of three shards completed, 19,838 tests passed, 0 failed (the third was stopped for host memory). Across 170,432 carry-over decisions, 473 changed a value, all of one kind: a monthly input stored for January, and a later month the run had already calculated (December). Master returned the default for the months in between; this PR carries the January input.
    • Variables: ssi_lives_in_medical_treatment_facility, ssi_medicaid_pays_majority_of_care, receives_ssi.
    • Tests: 8 files (Kansas SSPP, Indiana SSP, California San Bernardino general relief, Texas CEAP). Rerun alone, all 8 pass on master and on this PR: 61 tests each.
    • The third shard, and running policy/reform in one process, run out of memory on stock master too (reform/ passed a 12 GB footprint within 205 s on b78b0ba; both cores grew about 0.6 GB per completed reform test: 12.0 GB after 21 tests on master, 8.1 GB after 13 on this PR). A separate investigation into the YAML test runner's memory growth owns that; it is not caused by this PR.

Multi-year in one simulation (3,000-household subsample):

  • 2025 then 2026: master gives 2026 income tax $32.87bn (the monthly age/12 carried by Carry over only values stored at the variable's definition period #557's bug), against $1,962.456bn for 2026 alone. This PR gives $1,962.456bn either way, along with state income tax $494.342bn and household net income $14,012.652bn. What still differs from the single-year run is float32 drift in uprated values (weights and employment income about 1e-7 relative; fixed by Uprate only from inputs, so uprated values don't depend on calculation order #563, stacked on this PR), plus a few tax units in the persistent itemization branches (fixed by policyengine-us#9738).
  • 2025, 2026, then 2027: master gives 2027 income tax $31.72bn and Carry over only values stored at the variable's definition period #557 alone gives $2,038.34bn (equal to 2027 alone). This PR raises in 2027 (ParameterNotFoundError ... md.msde.ccs.payment.informal.rates.UNKNOWN): in the itemizing/not_itemizing branches forked at 2025 and reused at 2027, 27 Maryland households get county UNKNOWN.
    • The cause is policyengine-us's county formula. It emulates carry-over by returning holder.get_array(sorted(holder.get_known_periods())[-1]) from the default branch, which inside a branch reused across years returns None.
    • Core then fell back to carry-over. Master and Carry over only values stored at the variable's definition period #557 carry an earlier calculated county; this PR carries only inputs, and the eCPS stores county_fips, not county.
    • Land the policyengine-us fix (read the branch's own latest stored county, or fall through to county_fips), or policyengine-us#9738's per-period branches, before or with the core release that includes this PR. Production paths (policyengine.py, the APIs) build one simulation per year and are unaffected.

Behaviour changes

  • Sparse inputs carry forward. Monthly inputs for January and June now give January's value for February to May (master: the default). Each period uses the latest earlier input, as a January-only input already did. A package that wants gaps to default needs explicit gap values.
  • Carried and calculated values no longer carry from one another; results now equal what the period gives alone. One visible side effect: with an input for 2011 and a later stored period (say 2014), calculating 2012 now carries the 2011 input and caches it, so get_array(2012) is set; master returned the default uncached. With no input to carry, caching is exactly as on master.
  • A helper-less variable with an own-unit input and a later input in another unit carries the own-unit one (as Carry over only values stored at the variable's definition period #557, and as before 3.24.0); no country package declares set_input = None.

After #578 (head 2580d43b)

#578 merged while this PR was open and conflicted with it in in_memory_storage.py. The Fix section above describes the head round 4 reviewed (912a4ac5). Three commits follow it. None changes which input carries for a period with a finite end.

  • 12445111: merge of master. Give a storage a set of shared keys only while it shares an array #578 gives a storage a set of shared keys only while it shares an array, because an empty set in every InMemoryStorage cost about 1.3 MB per policyengine-us simulation and ran PE-US CI out of memory (3.32.12 gives every holder's storage an empty set: about 1.3 MB more per policyengine-us simulation #577). This PR had added a second set to every storage and a __setstate__ that gave every unpickled storage its own _shared set, which Give a storage a set of shared keys only while it shares an array #578's tests reject. The merge keeps Give a storage a set of shared keys only while it shares an array #578's code and drops that __setstate__.
  • d89a2ea9: the in-memory storage records its inputs, not its derived values. is_derived is now "stored and not an input". A simulation calculates far more values than it is given: in a policyengine-us household, 5,201 of 5,204 stored values are derived. A storage has a set of inputs only while it holds one, and it shares one frozenset with its clones until either changes its own. A pickled state records that inputs were recorded; a state from before counts every stored value as an input, as it did then. The on-disk storage keeps its set of derived keys.
  • 6ddbaf9f, 2580d43b: periods ending after 9999-12-31. The carry-over key called period.stop, which raises for a period that ends after the last date datetime has (day:9999-12-30:3), where master carried the input. The key now uses _end_order: the number of the day after the period's last day, by integer arithmetic. It equals stop plus one day wherever stop has a value, and orders every other period by its true end.

Retained memory per simulation. Measured on a policyengine-us 2.18.3 household with one household_net_income calculation: 6,185 storages, and 30,925 holder clones across 5 branches.

Core Retained Storages with an index set of their own
master with #578 20.94 MB 0
merge, one set of derived keys per storage that needs one 24.35 MB 1,514
same, shared with clones 22.57 MB 1,514
this head (inputs recorded) 21.25 MB 3 (648 bytes)

The remaining 0.31 MB is one more 8-byte attribute slot in each of the 37,110 storage objects.

Results unchanged. policyengine-us, eCPS 2024 (us-data 1.112.3), 3,000-household subsample, 2026: 25 of 25 arrays bitwise identical between master cbfdedf7 and 6ddbaf9f. 2580d43b changes only _end_order, which the round-2 review checked against the old key on 440,000 candidate sets of positive-sized periods with a finite end: all matched.

Tests.

  • tests/core/test_storage_input_index.py pins the input index.
  • Give a storage a set of shared keys only while it shares an array #578's differential (test_storage_shared_index_differential.py) now also stores derived values and compares is_derived with a storage that always had both sets.
  • Regressions and properties cover the end order, checked against Period.stop and against numpy's calendar.
  • Full suite on 2580d43b: 1252 passed, 4 skipped, 1 xfailed. Country-template YAML: 39 passed. CI: 18 of 18.

Reviews of the delta (GPT-6.1 Sol, hard tier):

  • Round 1, on 6ddbaf9f: REQUEST CHANGES. Periods ending after 9999-12-31 all sorted equal. Fixed in 2580d43b. It found the storage change sound over 60,000 operations against an independent model.
  • Round 2, on 2580d43b: APPROVE WITH NITS. It checked 574,173 endpoints against an independent calendar model, and every earlier surviving mutant is now killed.
  • Its two nits:
    1. A day period of size zero or less (outside the Period contract of positive sizes) now ends before a one-day period, where stop treated it as one day.
    2. The ETERNITY test would accept a large finite value. Production returns infinity; the assertion is tightened in a follow-up.

axiom: n/a: engine fix to carry-over in policyengine-core; no policy rule changes

🤖 Generated with Claude Code

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>
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Like-for-like PE-US benchmark: core #557 vs #562. Setup: Enhanced CPS 2024, 3,000-household subsample (fixed seed), policyengine-us main 20ccd5ac with and without policyengine-us#9738. Each cell compares all 24 benchmark arrays bitwise (income tax, itemization, both branch liabilities, CTC, EITC, DE/ID/VA and state income tax, household net income, SNAP and more).

Comparison core master b78b0ba (with #556) #557 at 7af22b1 #562 at a80a3b9
2024 single-year vs master n/a identical identical
2025 single-year vs master n/a identical identical
2025 from a 2024-then-2025 run vs 2025-only, PE-US main 20 of 24 differ (age at 1/12: 1,114 tax units' income tax change) 8 differ (formula-branch residual: 124 tax units' ctc_limiting_tax_liability, 1 tax unit's income tax) 8 differ (same residual)
Same, with PE-US #9738 n/a identical identical
#557 vs #562, 2025 from the two-year run with #9738 n/a identical identical

So on this model the two core PRs are interchangeable. Both remove the age/12 error, both leave single-year results bitwise unchanged, and both need PE-US #9738 for the second year to match exactly. They differ in scope:

Scripts and arrays: ~/reviews/pe-us-second-year-2026-10-01/cmp_bench.sh, rerun_a2.sh, cmp/ on the investigation host.

MaxGhenis and others added 6 commits October 2, 2026 02:54
…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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
…he 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>
…ebuilds

Closes three mutants the suite did not kill.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
@MaxGhenis
MaxGhenis marked this pull request as ready for review October 2, 2026 08:52
MaxGhenis added a commit that referenced this pull request Oct 2, 2026
…s as stored

Round-2 review fixes:
- An entry is dropped only when neither storage still holds its value, so an
  input deleted from memory that survives on disk stays an input.
- Twelve months starting on the first of a month are recorded as the year
  storage keys them under, so deleting that year drops the entry.
- Deleting compares the keys each storage holds before and after: it no
  longer goes through the record or loads files for a disk-backed holder.
- The property model is seeded from the situation, checks to_input_dataframe
  itself, and a second property covers memory and disk storage.
- Regression for the carry-over case found in the review of #562.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis added a commit that referenced this pull request Oct 3, 2026
Two earlier inputs in a variable's own unit can start on the same day:
Holder.set_input stores a year:2012:2 input to a yearly variable as
given (a set_input helper is only called for another unit), beside one
for 2012. The uprating source was max(..., key=start), so the tie went
to whichever was stored first, or to the one in memory over the one on
disk: 2015 came out as [1.115, 1.115] or [5.576, 5.576] by set order.

Move #562's carry-over sort key into _latest_input_key and use it for
the uprating source too: on a tie, the input that ends last, then the
larger unit, then the period's string form. Distinct periods never tie,
so the source depends only on the stored inputs. The factor still runs
from the source's start, and the candidates are unchanged (own unit,
starting before the period, inputs only).

Tests: regressions for both set orders with and without a set_input
helper, monthly inputs, a 10-year input whose string sorts first, memory
versus disk and a branch input; Hypothesis properties for set-order and
disk-placement independence and for flat-index uprating agreeing with
auto-carry-over; the fixture's reference rule breaks ties the same way.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis and others added 2 commits October 3, 2026 10:12
…a set per storage

#578 gave a storage a set of shared keys only while it shares an array: an
empty set in every InMemoryStorage cost 216 bytes per variable per
simulation, about 1.3 MB per policyengine-us simulation, and ran PE-US's CI
batches out of memory (#577). This branch's derived marks added a second
empty set to every storage, and its __setstate__ gave every unpickled
storage its own _shared set, which #578's pickle tests reject.

Resolve by keeping #578's code and giving _derived the same treatment:
_NOTHING_DERIVED, a class-level empty frozenset, until a storage holds a
derived value; its own set from the first derived put; released with one
dict pop when the last derived key is replaced by an input or deleted. The
class attributes cover pickles from before either set existed, so
__setstate__ goes. A clone copies only the marks of keys it holds.
apply_reform's wipe releases the marks instead of assigning an empty set.

Tests: #578's differential now also stores derived values and compares the
effective marks and is_derived with the eager storage; new
test_storage_derived_index.py pins the lazy marks (inputs never need a set,
release on replace/delete/wipe, clones, copies, old pickles, simulation and
apply_reform).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h 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>
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>
MaxGhenis added a commit that referenced this pull request Oct 3, 2026
Two earlier inputs in a variable's own unit can start on the same day:
Holder.set_input stores a year:2012:2 input to a yearly variable as
given (a set_input helper is only called for another unit), beside one
for 2012. The uprating source was max(..., key=start), so the tie went
to whichever was stored first, or to the one in memory over the one on
disk: 2015 came out as [1.115, 1.115] or [5.576, 5.576] by set order.

Move #562's carry-over sort key into _latest_input_key and use it for
the uprating source too: on a tie, the input that ends last, then the
larger unit, then the period's string form. Distinct periods never tie,
so the source depends only on the stored inputs. The factor still runs
from the source's start, and the candidates are unchanged (own unit,
starting before the period, inputs only).

Tests: regressions for both set orders with and without a set_input
helper, monthly inputs, a 10-year input whose string sorts first, memory
versus disk and a branch input; Hypothesis properties for set-order and
disk-placement independence and for flat-index uprating agreeing with
auto-carry-over; the fixture's reference rule breaks ties the same way.

Review (GPT-6.1 Sol, f051cc2): the key's period.stop raised
OverflowError for a period ending after 9999-12-31 (day:9999-12-30:3),
where the start-only key did not. The key now orders ends with #562's
_end_order, which puts such a period last; regressions use a daily
variable. The flat-uprating differential now skips only the case where
the two paths really diverge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
MaxGhenis added a commit that referenced this pull request Oct 3, 2026
Two earlier inputs in a variable's own unit can start on the same day:
Holder.set_input stores a year:2012:2 input to a yearly variable as
given (a set_input helper is only called for another unit), beside one
for 2012. The uprating source was max(..., key=start), so the tie went
to whichever was stored first, or to the one in memory over the one on
disk: 2015 came out as [1.115, 1.115] or [5.576, 5.576] by set order.

Move #562's carry-over sort key into _latest_input_key and use it for
the uprating source too: on a tie, the input that ends last, then the
larger unit, then the period's string form. Distinct periods never tie,
so the source depends only on the stored inputs. The factor still runs
from the source's start, and the candidates are unchanged (own unit,
starting before the period, inputs only).

Tests: regressions for both set orders with and without a set_input
helper, monthly inputs, a 10-year input whose string sorts first, memory
versus disk and a branch input; Hypothesis properties for set-order and
disk-placement independence and for flat-index uprating agreeing with
auto-carry-over; the fixture's reference rule breaks ties the same way.

Review (GPT-6.1 Sol, f051cc2): the key's period.stop raised
OverflowError for a period ending after 9999-12-31 (day:9999-12-30:3),
where the start-only key did not. The key now orders ends with #562's
_end_order, which puts such a period last; regressions use a daily
variable. The flat-uprating differential now skips only the case where
the two paths really diverge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis
MaxGhenis merged commit 58e23f5 into master Oct 3, 2026
18 checks passed
@MaxGhenis
MaxGhenis deleted the fix-carry-over-order branch October 3, 2026 19:31
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Merged at the reviewed head 2580d43b (squash 58e23f56). Audit trail:

  • Decision: d820, ruled yes by Max on 2026-10-02: merge with or after Calculate each year's formula branches fresh so multi-year simulations match single-year ones policyengine-us#9738, which merged 2026-10-03 14:11Z.
  • Reviews:
    • Round 4 (GPT-6.1 Sol) approved 912a4ac5.
    • Delta round 1 on 6ddbaf9f: REQUEST CHANGES. Periods ending after 9999-12-31 all sorted equal. Fixed in 2580d43b.
    • Delta round 2 on 2580d43b: APPROVE WITH NITS.
  • The two nits, both P3:
    1. A day period of size zero or less now ends before a one-day period, where Period.stop treated it as one day. Such sizes are outside the Period contract (positive sizes), so no change.
    2. The ETERNITY test would accept a large finite value. Production returns infinity; the assertion is tightened in a follow-up.
  • Checks on 2580d43b:
    • CI: 18 of 18.
    • Local: 1252 passed, 4 skipped, 1 xfailed; country-template YAML: 39 passed.
  • Results: policyengine-us eCPS 2024 (us-data 1.112.3), 3,000-household subsample, 2026: 25 of 25 arrays bitwise identical between master cbfdedf7 and 6ddbaf9f. 2580d43b changes only the end order, which round 2 matched against the old key on 440,000 candidate sets.
  • Memory: 21.25 MB retained per policyengine-us household simulation, against 20.94 MB on master with Give a storage a set of shared keys only while it shares an array #578.

#563 and #582 follow.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant