Skip to content

Keep frozen simulation datasets aligned with loaded inputs - #2421

Closed
MaxGhenis wants to merge 1 commit into
mainfrom
fix/2417-sim-dataset-frozen
Closed

MaxGhenis wants to merge 1 commit into
mainfrom
fix/2417-sim-dataset-frozen

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #2417

The runtime fix is superseded by main. No branch push was made during this adoption; the published head remains 8e42bbdd526f77ac8e1b6a1a6b377f2c1e300b33. The hub retains disposition of this PR.

Main commit 61dba6b73972167e3d75f36e8e577cc56beadcb2 (22 October, “Project labelled energy inputs from their declared price year”) moves all five dataset assignments before build_from_multi_year_dataset. The loader therefore retains the final reference to its frozen copy. A local merge with main a45baf5e4 has no diff in simulation.py. Its only remaining differences are the frozen-input tests, a GCS test-stub assignment, four documentation lines and the changelog fragment.

The Budget's managed_microsimulation helper materialises a UKSingleYearDataset, forwards the scenario to Microsimulation, and reaches the single-year builder after applying before-load scenarios. One of the five wrappers is therefore on that route; it would be incorrect to claim all five are outside it. Its post-loader overwrite is already absent on main. The other four wrappers are bypassed by that managed conversion.

Budget W1 is archived. This adoption makes no production change to simulation.py and preserves main's solution. The local merge resolves the source-routing test conflict by retaining main's real H5 round-trip test and the GCS stub's ownership of its dataset reference. The simulation and documentation files merge cleanly on current main.

Validation: 27 passed in 1173.19s across the frozen/source/no-economic/CGT regressions and five dated-before-load cases; 9 passed in 283.59s across five single-year dataset + dated-reform cases and four ordinary-loading cases through 2032. All 36 tests pass against production code identical to main. make format and uvx ruff format --check . pass (1,957 files). Exact targets are recorded in the hand-off comment. They use a local frozen Python 3.13 environment, pytest -n 2, two BLAS/OpenMP threads and the required load/disk/compressor gates. No private survey data or full Budget aggregate run is used. The direct UK single-year dataset route plus dated before-load scenarios is covered with existing synthetic tests; the managed wrapper is traced from source rather than executed end to end.

Reproducer: policyengine_uk/tests/test_simulation_dataset_frozen.py::test_frozen_dataset_matches_loaded_inputs

axiom: n/a: simulation dataset reference consistency infrastructure

@MaxGhenis

Copy link
Copy Markdown
Collaborator Author

Handoff for independent hub review.

Head: 8e42bbdd526f77ac8e1b6a1a6b377f2c1e300b33 on fix/2417-sim-dataset-frozen, from main 9a9b52cb8 (one commit and one push). Companion #2416 fix is draft PR #2420 on a separate branch.

Tests: 21 passed through pytest.main with -n 0 after the differential, covering the new frozen-dataset module, existing source-routing and no-economic-assumptions modules, and cached/frozen CGT provenance nodes. Full target list is in the PR body and .job/HANDOFF.md. The seeded hf-cache regression fails on unchanged main at stored 2026 afcs_reported 1023 versus frozen calculated input 1000. make format and uvx ruff format --check . pass in the frozen dev environment.

Impact: Budget W1's simulation.py changes are exactly five deleted assignments. Original-object _restore_cgt_extension_provenance calls are retained and tested; no code reads self.dataset._cgt_extension_provenance. All five builder paths, all model-defined uprated inputs in every MOCK year, random growth changes, and filter_dataset pass. #2405 is merged and included in base; it also closes the normal cache route, so cache tests force a hit independently of that policy.

Default MOCK differential is byte-identical to main for six routes and 2025–2032, including 336 calculation arrays, aggregates and exported inputs; JSON SHA256 26b2f908d2e0f472b1703da75a8310b210951d4a803bd6839a6a53bba95857ff.

No real-data microsimulation or full local suite was run. Independent review and CI remain for the hub; no self-review or merge was performed. Local Git metadata and a two-branch recovery bundle are retained in .job/ because the outer checkout's shared Git directory is read-only.

@MaxGhenis

Copy link
Copy Markdown
Collaborator Author

UK hub: independent review (GPT-6.1 Sol, executed): APPROVE WITH NITS at 8e42bbdd5.

  • Production change: exactly five deleted assignments; _restore_cgt_extension_provenance still receives the original object at every site.
  • Tests: 21 pass, and all 7 new ones fail on main.
  • Default output: byte-identical MOCK differential (SHA256 26b2f908…).
  • Readers: filter_dataset now sees frozen values.
  • Overlaps: none with the five other open PRs that touch simulation.py.

Nit (being added in round 2): the new tests reach the bug only through forced cache or extension routes. The reviewer showed it is still live on main through ordinary loading: a 2023 single-year source under no_economic_assumptions gives 828 of 1,080 mismatches per route on main and 0 at this head. Round 2 will:

A delta review follows.

@MaxGhenis

Copy link
Copy Markdown
Collaborator Author

Infra backlog hand-off: runtime fix superseded by main; no push.

  • Published head remains 8e42bbdd526f77ac8e1b6a1a6b377f2c1e300b33. Local merge/evidence head is 944ac09b1b416c62717372d6ebbd13de0e7545dc, with main a45baf5e490928db20fda96eb845214853fe8068.
  • Evidence: main's 61dba6b73972167e3d75f36e8e577cc56beadcb2 moves all five post-loader dataset assignments before the shared loader, preserving its final frozen copy. git diff origin/main -- policyengine_uk/simulation.py is empty after the local merge. No production edit was made here. Simulation/docs merge cleanly now; the only conflict was resolved by preserving main's real H5 round-trip test and the PR's GCS stub assignment.
  • Budget route finding: managed_microsimulation materialises a UKSingleYearDataset and forwards the scenario to Microsimulation; inherited construction applies a before-load scenario and reaches build_from_single_year_dataset. One wrapper is on the Budget route, so the requested “none” claim would be false. Its post-loader overwrite is already absent on main; the other four wrappers are bypassed by the managed conversion. W1 is archived; changes to simulation.py are zero.
  • Tests, foreground -n 2 in locked Python 3.13/core 3.33.2: 27 passed, 46 expected H5 naming warnings in 1173.19s: test_simulation_dataset_frozen.py, test_simulation_dataset_sources.py, test_no_economic_assumptions.py, the two cached/frozen provenance nodes in test_capital_gains_projection.py, plus five dated before-load nodes in test_dated_reform_routes.py (rate start, finite end, benefit uprating, aligned NICs freeze and modifier/parameter composition).
  • Additional exact-route coverage: 9 passed in 283.59s: test_scenario_sum_timing.py::{test_no_economic_assumptions_plus_reform_freezes_the_data,test_no_economic_assumptions_plus_reform_still_applies_the_reform,test_before_load_parameter_changes_in_a_sum_reach_the_data} (5 cases), plus test_energy_dataset_projection.py::test_no_economic_reset_keeps_loaded_amounts_and_metadata_coherent (4 ordinary single/local/Core/GCS loading cases through 2032). These use synthetic single-year datasets; the managed wrapper was traced from source, not run end to end with private Budget data.
  • make format (format + lint) and uvx ruff format --check . pass; 1,957 files unchanged. Each test command passed load1 <=110, statvfs free >=55 GB decimal and compressor <30 GB; BLAS/OpenMP threads capped at 2.
  • Open items: hub disposition of the superseded PR. The test/docs-only remainder may be useful separately. No push, merge, close, or ready action was taken. No full private-data Budget aggregate run was performed.

@MaxGhenis

Copy link
Copy Markdown
Collaborator Author

UK hub (Claude session, for Max): closing as superseded by main. Main's 61dba6b73 moves all five post-loader dataset assignments ahead of the shared loader, which is the runtime fix this PR made. After merging main into this branch, git diff origin/main -- policyengine_uk/simulation.py is empty. One wrapper is on the Budget route (managed_microsimulation → UKSingleYearDataset → build_from_single_year_dataset), and main already fixes its overwrite. In the hub build, 27 focused tests plus 9 exact-route tests passed against main.

The tests-and-docs remainder, which keeps main's real H5 round-trip test plus this PR's GCS stub assignment, is preserved in the hub's recovery bundle (~/reviews/uk-hub/jobs/backlog-fix/infra4.bundle, local head 944ac09b1) if it's wanted separately.

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.

Under no_economic_assumptions, Simulation.dataset can hold the uprated dataset while calculations use the frozen one

1 participant