diff --git a/changelog.d/954.changed.md b/changelog.d/954.changed.md new file mode 100644 index 000000000..ad5a9a9fd --- /dev/null +++ b/changelog.d/954.changed.md @@ -0,0 +1 @@ +Made only US state runs compute congressional district impacts; national runs (no region, or `us`) and region-group runs, and so segmented national runs, their reduce and Stage 12 aggregation of a national report, now compute none: `congressional_district_impact` is null in their output, and the gateway's job-status response leaves that null field out (Stage 12 aggregation of a state report spelled `state/...` still computes that state's districts), because nationwide reports take each district from a separate run of its state on that state's dataset, which matches the state's own report; the beta integration check of a US national result now expects no `congressional_district_impact`, whether absent or null, and the state checks still expect the state's districts. diff --git a/projects/policyengine-apis-integ/tests/simulation/test_calculate.py b/projects/policyengine-apis-integ/tests/simulation/test_calculate.py index 105d7bcb6..d61607a3b 100644 --- a/projects/policyengine-apis-integ/tests/simulation/test_calculate.py +++ b/projects/policyengine-apis-integ/tests/simulation/test_calculate.py @@ -34,7 +34,6 @@ "no_change_percentage", "population", } -_AT_LARGE_DISTRICT_IDS = {"AK-01", "DC-01", "DE-01", "ND-01", "SD-01", "VT-01", "WY-01"} # From the generated client, so a stage added to the schema is accepted here. _JOB_STAGES = frozenset(stage.value for stage in JobStage) @@ -238,14 +237,30 @@ def assert_congressional_district_results( ) if expected_district_ids is not None: - district_ids = set(_district_ids(district_results)) - missing = expected_district_ids - district_ids - assert not missing, ( - f"Missing expected district IDs {sorted(missing)} " - f"from result IDs: {sorted(district_ids)}" + district_ids = _district_ids(district_results) + # Sorted lists, not sets, so a district listed twice fails too. + assert sorted(district_ids) == sorted(expected_district_ids), ( + f"Expected district IDs {sorted(expected_district_ids)}, " + f"got {sorted(district_ids)}" ) +def assert_no_congressional_district_results(economy_result: object) -> None: + """Check a US national result has no congressional_district_impact. + + Only state runs compute districts: nationwide reports take them from a + separate run of each state. The field is null in the run's output, and the + gateway's job-status response leaves null fields out, so a deployed result + has no such field. + """ + economy_result = _as_mapping(economy_result, label="economy result") + district_impact = economy_result.get("congressional_district_impact") + assert district_impact is None, ( + "Expected no congressional_district_impact in a US national result, " + f"got {district_impact!r}" + ) + + _UK_AREA_RESULT_KEYS = ("constituency_impact", "local_authority_impact") _PUBLISHED_DISCLOSURE_TIERS = {"reported", "approximate"} @@ -323,6 +338,74 @@ def test_result_assertions_accept_generated_client_objects() -> None: assert_congressional_district_results(result, expected_district_prefix="UT-") +def _state_result_with_districts(*district_ids: str) -> dict: + return { + "congressional_district_impact": { + "districts": [ + { + "district": district_id, + "average_household_income_change": 0.0, + "relative_household_income_change": 0.0, + "winner_percentage": 0.0, + "loser_percentage": 0.0, + "no_change_percentage": 1.0, + "population": 1.0, + } + for district_id in district_ids + ] + } + } + + +def test_state_assertions_accept_an_at_large_state_with_its_one_district() -> None: + assert_congressional_district_results( + _state_result_with_districts("VT-01"), + expected_district_prefix="VT-", + expected_district_ids={"VT-01"}, + ) + + +@pytest.mark.parametrize( + "district_ids", + [("VT-00",), ("VT-01", "VT-02"), ("VT-01", "VT-01")], + ids=["another-id", "an-extra-district", "a-repeated-district"], +) +def test_state_assertions_reject_other_district_ids(district_ids) -> None: + with pytest.raises(AssertionError, match="district IDs"): + assert_congressional_district_results( + _state_result_with_districts(*district_ids), + expected_district_prefix="VT-", + expected_district_ids={"VT-01"}, + ) + + +def test_national_us_assertions_accept_a_result_without_districts() -> None: + assert_no_congressional_district_results( + _GeneratedClientObject({"budget": {}, "congressional_district_impact": None}) + ) + assert_no_congressional_district_results({"congressional_district_impact": None}) + # The gateway's job-status response leaves null fields out, so a deployed + # national result has no district field at all. + assert_no_congressional_district_results({"budget": {}}) + assert_no_congressional_district_results(_GeneratedClientObject({"budget": {}})) + + +@pytest.mark.parametrize( + "district_impact", + [ + {"districts": []}, + {"districts": [{"district": "UT-01"}]}, + _GeneratedClientObject({"districts": [{"district": "UT-01"}]}), + ], + ids=["no-districts-listed", "districts-listed", "generated-client-object"], +) +def test_national_us_assertions_reject_district_results(district_impact) -> None: + with pytest.raises(AssertionError, match="congressional_district_impact"): + assert_no_congressional_district_results( + {"congressional_district_impact": district_impact} + ) + + _BREAKDOWN_CONSTITUENCY = { "constituency_code": "E14001063", "constituency_name": "Aldershot", @@ -613,9 +696,10 @@ def test_calculate_default_model( ): """A simulation on the default model version returns economic impact data. - Given a simulation request with default model version + Given a US national simulation request with default model version When the simulation is submitted and polled to completion - Then the result contains expected economic impact data. + Then the result contains expected economic impact data and no + congressional districts, which only state runs compute. """ # Given request = SimulationRequest.from_dict( @@ -649,10 +733,7 @@ def test_calculate_default_model( economy_result = result.result assert_economy_result_sections(economy_result) - assert_congressional_district_results( - economy_result, - expected_district_ids=_AT_LARGE_DISTRICT_IDS, - ) + assert_no_congressional_district_results(economy_result) @pytest.mark.beta_only @@ -719,16 +800,18 @@ def test_calculate_specific_model( ): """A simulation pinned to a specific US model version returns impact data. - Given a simulation request with a specific model version + Given a simulation request with a specific model version, for Vermont When the simulation is submitted and polled to completion - Then the result contains expected economic impact data. + Then the result contains expected economic impact data, and Vermont's one + at-large district: nationwide reports take each at-large district from its + state's own run, as here. """ # Given request = SimulationRequest.from_dict( { "country": "us", "version": us_model_version, - "region": "state/ut", + "region": "state/vt", "scope": "macro", "reform": { "gov.irs.credits.ctc.refundable.fully_refundable": { @@ -763,7 +846,8 @@ def test_calculate_specific_model( assert_economy_result_sections(economy_result) assert_congressional_district_results( economy_result, - expected_district_prefix="UT-", + expected_district_prefix="VT-", + expected_district_ids={"VT-01"}, ) diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/segmented_national_reduce.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/segmented_national_reduce.py index 26ba85cac..4d059e96f 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/segmented_national_reduce.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/segmented_national_reduce.py @@ -142,8 +142,9 @@ def build_national_output( """Reduce the children's microdata into the full national macro output dict. Runs the existing ``SimulationOutputBuilder`` over stand-in simulations - with ``resolved_region_code=country`` so the national congressional - district table populates. No model recompute occurs. + with ``resolved_region_code=country``, as for any national run, so the + output has no congressional districts: nationwide reports take those from + separate state runs. No model recompute occurs. """ country = inputs.country country_module = inputs.country_module diff --git a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py index 4ce4a49b8..7f76ec7d3 100644 --- a/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py +++ b/projects/policyengine-simulation-executor/src/policyengine_simulation_executor/simulation_output_geographic.py @@ -223,12 +223,19 @@ def _integer_record_value(record: Mapping[str, Any], key: str) -> int | None: def _should_build_us_congressional_district_impact(region_code: str | None) -> bool: - if region_code is None: - return True - normalized_region_code = region_code.lower() - return normalized_region_code == "us" or normalized_region_code.startswith( - ("state/", "region_group/") - ) + """Return whether a US run for this region computes congressional districts. + + Only state runs do, such as ``state/ca`` in any case. National runs (no + region, or ``us``) and region-group runs compute none, so neither do + segmented national runs, their reduce or Stage 12 aggregation of a + national report: a nationwide report takes its districts from a separate + run of each state on that state's dataset, which gives each district the + same figures as its state's own report. Stage 12 aggregation of a state + report still computes that state's districts when the request spells the + region ``state/...``: that path passes the region as sent, so a bare code + such as ``ut`` computes none there. District and place runs compute none. + """ + return region_code is not None and region_code.lower().startswith("state/") def build_congressional_district_impact( @@ -238,10 +245,11 @@ def build_congressional_district_impact( *, region_code: str | None = None, ) -> CongressionalDistrictImpactOutput | None: - """Compute US congressional district impacts for national, state or group runs. + """Compute US congressional district impacts for a state run. - None for other countries or regions, and when the calculation fails (the - failure is logged; SPM selection errors still propagate). + None for other countries and other regions, including national and + region-group runs, and when the calculation fails (the failure is logged; + SPM selection errors still propagate). """ if country != "us": return None diff --git a/projects/policyengine-simulation-executor/tests/test_region_group_resolution.py b/projects/policyengine-simulation-executor/tests/test_region_group_resolution.py index 3c366b068..01d3830b4 100644 --- a/projects/policyengine-simulation-executor/tests/test_region_group_resolution.py +++ b/projects/policyengine-simulation-executor/tests/test_region_group_resolution.py @@ -1,4 +1,4 @@ -"""Unit tests for region-group resolution (B2) and the CD-impact gate (B3).""" +"""Unit tests for region-group resolution (B2) and the district-impact regions (B3).""" from unittest.mock import MagicMock @@ -11,7 +11,7 @@ from policyengine_simulation_executor import region_resolution from policyengine_simulation_executor import simulation_runtime as sr from policyengine_simulation_executor.simulation_output_geographic import ( - _should_build_us_congressional_district_impact as gate, + _should_build_us_congressional_district_impact as should_build_districts, ) @@ -119,13 +119,16 @@ def test__resolve_region_dispatches_to_group(self, monkeypatch): assert isinstance(res.scoping_strategy, RegionGroupStrategy) -class TestCongressionalDistrictGate: - def test__allows_national_state_and_group(self): - assert gate(None) is True - assert gate("us") is True - assert gate("state/ca") is True - assert gate("region_group/state/hi+state/ia") is True +class TestCongressionalDistrictRegions: + def test__allows_states(self): + assert should_build_districts("state/ca") is True + assert should_build_districts("STATE/CA") is True + + def test__rejects_national_and_group(self): + assert should_build_districts(None) is False + assert should_build_districts("us") is False + assert should_build_districts("region_group/state/hi+state/ia") is False def test__rejects_district_and_place(self): - assert gate("congressional_district/CA-01") is False - assert gate("place/CA-12345") is False + assert should_build_districts("congressional_district/CA-01") is False + assert should_build_districts("place/CA-12345") is False diff --git a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py index 7166450fb..b75e5e92a 100644 --- a/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py +++ b/projects/policyengine-simulation-executor/tests/test_simulation_output_builder.py @@ -199,6 +199,51 @@ def fake_build_congressional_district_impact( assert calls == [("us", baseline, reform, "state/ut")] +@pytest.fixture +def district_calculations(monkeypatch): + """Record each call to policyengine.py's district calculation, in its place.""" + calculations = [] + + def record_district_calculation(baseline, reform): + calculations.append((baseline, reform)) + return SimpleNamespace(district_results=[]) + + monkeypatch.setattr( + "policyengine.outputs.congressional_district_impact." + "compute_us_congressional_district_impacts", + record_district_calculation, + ) + return calculations + + +@pytest.mark.parametrize( + "region_code", + [None, "us", "region_group/state/hi+state/ia"], +) +def test_builder_skips_districts_for_national_and_region_group_runs( + district_calculations, region_code +): + baseline, reform = macro_baseline_reform() + builder = make_output_builder( + "us", baseline, reform, BuilderOptions(resolved_region_code=region_code) + ) + + assert builder._build_congressional_district_impact() is None + assert district_calculations == [] + + +def test_builder_builds_districts_for_state_runs(district_calculations): + baseline, reform = macro_baseline_reform() + builder = make_output_builder( + "us", baseline, reform, BuilderOptions(resolved_region_code="state/ut") + ) + + assert builder._build_congressional_district_impact() == ( + CongressionalDistrictImpactOutput(districts=[]) + ) + assert district_calculations == [(baseline, reform)] + + def test_builder_records_output_timings_without_real_calculation(monkeypatch): baseline = object() reform = object()