Eliminate redundant dataset deep-copies in multi-year extension (11x faster)#9000
Open
anth-volk wants to merge 4 commits into
Open
Eliminate redundant dataset deep-copies in multi-year extension (11x faster)#9000anth-volk wants to merge 4 commits into
anth-volk wants to merge 4 commits into
Conversation
pandas 3's always-on copy-on-write enables the shallow-copy dataset extension fast path added in the following commits. Python 3.9/3.10 (retained for Census, #8035) stay on pandas 2.x via environment markers, mirroring policyengine-uk's tables pin pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
USSingleYearDataset.copy and USMultiYearDataset.copy accept deep=False to produce shallow copies whose entity DataFrames share column buffers under pandas copy-on-write. Default remains deep=True, so existing callers are unchanged. Shallow callers must use whole-column assignment only; on pandas 2.x shallow copies alias mutably, which is why the fast path is gated on pandas 3 downstream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
extend_single_year_dataset deep-copied all six entity DataFrames once per projected year (2024-2035), and _apply_uprating deep-copied the entire multi-year set again — 23 full dataset copies, most of which were immediately overwritten by uprating or never modified at all. Year frames are now shallow copies of the base year when pandas copy-on-write is available (pandas >= 3): carried-forward columns share base-year buffers, and uprating's whole-column assignments materialize only the columns they replace. _apply_uprating gains a copy flag so the extension path can skip its defensive copy; external callers keep the copying default. On pandas 2.x (Python 3.9/3.10) the extension falls back to deep copies, preserving today's behavior. Measured on the full Populace 2024 dataset (12 years): median extension time 20.95s -> 1.90s (11x), with all 3,876 per-column checksums across all years bit-identical to the deep-copy path. Regression tests pin equivalence with a deep-copy reference, caller isolation, and the buffer sharing itself (so deep copies can't silently return). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g naming - Update extend_single_year_dataset and _apply_single_year_uprating docstrings to describe the shallow-copy fast path instead of the removed deep-copy mechanism. - Import _PANDAS_COW in test_dataset_copy.py instead of redefining it. - Test _apply_uprating(copy=False) in-place semantics, and add a forced-fallback test proving the pandas 2.x deep-copy path produces output identical to the fast path (the only CI coverage of the path Python 3.9/3.10 installs use). - Merge the two changelog fragments into one named after the branch per the changelog.d/<branch-name>.<type>.md convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PavelMakarchuk
approved these changes
Jul 15, 2026
PavelMakarchuk
left a comment
Collaborator
There was a problem hiding this comment.
Review (two focused reviewers: CoW correctness/aliasing, packaging/test quality)
Verdict: Approve — the shallow-copy mechanism is verified safe and the tests genuinely pin both the equivalence and the perf win.
✅ Verified
- No silent write-through paths on pandas ≥ 3 (checked empirically against pandas 3.0.0):
.valueson CoW views comes back read-only, so a raw in-place write raisesValueErrorloudly rather than corrupting sibling years;.loc/whole-column assignment are CoW-isolated. Traced the full consumer chain (extend_single_year_dataset→build_from_dataset→holder.set_input/InMemoryStorage.put) plus all in-repo mutators —set_input_divide_by_periodcopies first, the FLAT_FILE*=block operates on a copy, labor-supply blocks don't mutate frames. - Version gate is correct and fails safe: pandas 2 +
copy_on_write=Truestill deep-copies (only forfeits speed); the monkeypatched_PANDAS_COWglobal is read at call time so the fallback test is effective. _apply_uprating(copy=False)has no leaking call sites — and the base year now being copied too (datasets = [dataset.copy(...)]) fixes what on main was only safe by accident.- Tests would catch a regression:
test_extension_equals_deep_copy_referencerebuilds the old deep-copy path and compares every table/year withcheck_exact=True; the buffer-sharing test guards against deep copies silently creeping back. All fixture-based, no dataset download. - Markers/lock: PEP 508 markers are valid (no lexicographic version trap), 3.9/3.10 branch is reachable (
requires-python >=3.9), uv.lock consistent, no conflict with policyengine-core (pandas>=1). Downstream floors are permissive: policyengine.pypandas>=2.0.0, policyengine-us-datapandas>=2.3.1— no<3upper bounds, so the new floor resolves cleanly.
🟡 Worth stating explicitly / small follow-ups
- The
pandas>=3.0floor is stricter than the code requires — the_PANDAS_COWruntime gate already makes pandas 2.x fully correct, so the floor is a deliberate trade-off (guaranteed perf + CI on pandas 3, at the cost of force-upgrading downstream envs onto pandas 3's own breaking changes). Fine either way, but worth a sentence in the PR/changelog since a patch-level.changedbump understates a dependency tightening. - Behavior change to document: holder input arrays become read-only under pandas 3 (true on the deep path too, since
.valuesis a CoW view). Downstream code doingarr = holder.get_array(p); arr[mask] = xwill now raise where it previously worked — a one-liner in the changelog would save someone a confusing afternoon. - Cheap CI win: the py3.9/3.10 compat legs are install+smoke-import only, so real pandas-2 aliasing semantics (the thing the gate defends against) never run in CI. The new tests are fixture-based and fast — adding
pytest policyengine_us/tests/microsimulation/data/to those legs would cover it for seconds of CI time. copy(deep=False)is a public footgun on pandas 2: the safety gate lives ineconomic_assumptions, but the dataset classes are exported API — a py3.9/3.10 caller gets mutable aliasing with only a docstring warning. Consider raising or silently deepening insidecopy()when CoW is unavailable (which would also fix the mild layering smell of tests importing the private_PANDAS_COW).
Nice work on the equivalence methodology — the checksum verification plus the reconstructed-reference regression test is exactly how this kind of change should be validated.
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.
Fixes #9001
Summary
Microsimulation()setup profiling showed ~half of init time was spent deep-copying the full dataset ~23 times during multi-year extension:extend_single_year_datasetdeep-copies all six entity DataFrames once per projected year (2024–2035), then_apply_upratingdeep-copies the entire multi-year set again. Uprated columns are overwritten immediately after being copied; carried-forward columns never change at all.Under pandas copy-on-write (always on in pandas ≥ 3), year frames are now shallow copies of the base year: carried-forward columns share base-year buffers, and uprating's whole-column assignments materialize only the columns they replace.
_apply_uprating's redundant defensive copy is skipped on the extension path (external callers keep the copying default).Measured results (full Populace 2024 dataset, 12 years)
Microsimulation()initEquivalence was verified with
pd.util.hash_pandas_objectchecksums of every column of every entity table for every extended year, identical before/after.Impact on state-level runs
Each
Microsimulation()build sheds ~19s of redundant copying (init ~27s → ~8s). A reform analysis constructing baseline + reform simulations saves ~40s — roughly 13–33% of a 2–5 minute state-level run.Compatibility
pandas>=3.0on Python ≥ 3.11; Python 3.9/3.10 (retained for Census, Support Python 3.9 and 3.10 #8035) keeppandas>=2.0via environment markers — mirroring policyengine-uk'stablesmarker pattern.uv lockresolution is unchanged (2.3.3 / 3.0.0).*=,.loc) write through on 2.x, which is why the fast path is version-gated.holder._to_arraycasts float64→float32 atset_input, materializing fresh arrays, and pandas 3 hands out read-only.valuesviews.Tests
copy(deep=...)on both dataset classes._apply_uprating's default defensive copy.Commits
deepparameter on dataset copy methods (pure refactor, default unchanged)🤖 Generated with Claude Code