[v4.0.1] Fix contrib-CTC structural reforms in US microsim path#305
Merged
[v4.0.1] Fix contrib-CTC structural reforms in US microsim path#305
Conversation
Simulation(policy={"gov.contrib.ctc.*": ...}) was crashing at
.ensure() with 'NoneType has no attribute entity' because
_build_simulation_from_dataset instantiated entities against the
module-level policyengine_us.system (no user reform applied) while
building the population from the per-sim Microsimulation whose
tax_benefit_system DID have the structural reform applied. Reform-
registered variables like ctc_minimum_refundable_amount were then
absent from the population's entity registry at calc time.
Fix: pass microsim.tax_benefit_system to _build_simulation_from_dataset
instead of the module-level system. The per-sim system already has
all structural reforms (gov.contrib.ctc.minimum_refundable,
per_child_phase_in, per_child_phase_out) applied by
Microsimulation.__init__'s post-user-reform structural pass.
Real-world impact: Tara Watson's CTC+EITC expansion (policy 94589,
42 parameters including three gov.contrib.ctc.* in_effect gates)
now runs end-to-end on the v4.0 stack, producing the full -$87.9B
federal income tax impact for 2025 instead of crashing.
Regression guarded by tests/test_us_microsim_structural_reforms.py
which builds a tiny in-memory dataset, applies the minimum-refundable
reform via policy dict, and asserts the sim runs to completion.
Version bumped to 4.0.1.
Adds three additional tests to guard against the same class of bug: - Parametrized per-gate smoke test across all three gov.contrib.ctc structural-reform gates (minimum_refundable, per_child_phase_in, per_child_phase_out). Each activates cleanly through a dict reform. - Invariant test that the module-level policyengine_us.system stays pristine — structural reforms must only mutate the per-sim system. If someone refactors _build_simulation_from_dataset and accidentally points it back at the module system, both ends of the invariant fire. - "Reform parameter change reaches the output" test using a plain scalar parameter override (no structural-reform machinery), so regressions where ANY parameter-value reform silently becomes a no-op get caught, not just the contrib-CTC class. Confirmed each test fails when the fix is reverted; 3/6 catch the original bug (the two per_child_phase_* reforms only update existing variables so build_from_populations doesn't trip on them — kept as smoke tests for defense-in-depth). 403/403 tests pass with fix applied.
Code-simplifier: - Inline-lambda -> def _simple (tests/test_us_microsim_structural_reforms.py L68) — drops the noqa: E731 smell. - Tighten the fix's inline comment by ~3 lines, keep the load-bearing 'hide reform-registered variables' clause. Reproducibility-reviewer (blocker): - Replace brittle "module-level system stays pristine" check with a direct identity assertion: the TaxBenefitSystem passed into _build_simulation_from_dataset must be microsim.tax_benefit_system. Uses monkeypatch to capture the argument; passes a positive assertion that the captured system has the structural-reform variable registered. Resilient to upstream policyengine_us ever shipping that variable unconditionally. Reproducibility-reviewer (follow-ups): - test__reform_parameter_change_is_reflected_in_output now asserts on BOTH CTC (base-amount change) AND EITC (phase_in_rate change) so partial-application regressions are caught, not only no-op ones. - Added one non-CTC contrib gate (gov.contrib.streamlined_eitc) to the parametrised smoke test to prove the fix generalises beyond the CTC family. Verified fail-without-fix: reverting the one-line fix breaks 3/7 tests (same set as before) with clean errors — the invariant test now reports "system is not microsim.tax_benefit_system" instead of the raw NoneType traceback, which is the actual contract. 7 tests, all pass.
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.
Summary
Fixes a silent bug in the US microsim path that made dict-reforms involving
gov.contrib.ctc.*parameters crash at.ensure()with:Concretely: Tara Watson's published CTC+EITC reform (policy 94589 on app.policyengine.org) — 42 parameters with three
gov.contrib.ctc.*.in_effect = Truegates — was completely unable to run throughpe.us.economic_impact_analysison the v4.0 stack.Root cause
PolicyEngineUSLatest._build_simulation_from_datasetwas instantiating entities against the module-levelpolicyengine_us.system:But
Microsimulation.__init__applies structural reforms (triggered by user-reform parameter gates likegov.contrib.ctc.minimum_refundable.in_effect=True) to its owntax_benefit_system— not the module one. Building populations against the module system then left reform-registered variables likectc_minimum_refundable_amountinvisible at calc time;refundable_ctc(which had been swapped to the reform version) would then crash trying to sum them.Fix
Pass
microsim.tax_benefit_systemto_build_simulation_from_datasetinstead of the module-levelsystem. One-line change insrc/policyengine/tax_benefit_models/us/model.py.Evidence
On the v4.0 stack (pe-us 1.653.3 + us-data 1.73.0), running the full 42-param reform now produces:
vs. pe-us 1.601.0 (March 2026): −$69.6B (the +$18B drift is upstream model evolution, not a pe.py regression).
Tests
New
tests/test_us_microsim_structural_reforms.py— builds a tiny in-memory dataset, applies the minimum-refundable reform via dict, runs the sim end-to-end. Before the fix: crashes. After: passes.72/72 existing tests still pass.
Version
Bumped to 4.0.1 (patch — bug fix, no API changes).
🤖 Generated with Claude Code