From b11a9f8ae2ecc93f944e062c3328dbf1e1d6b01a Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 22 Sep 2026 10:07:29 -0400 Subject: [PATCH] Read the SPM poverty indicator as nullable in _person_rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_person_rate` computed `np.asarray(values) > 0` over a mask initialised all-True, so a missing indicator read as "not poor" and stayed in the denominator: every affected rate moved down by a real number in a JSON that published cleanly. A wholly unmeasured slice published `0.0` — a real-looking 0% poverty rate — without ever reaching the `denominator == 0` branch. Divide poor by observed instead: compare the indicator with 1, keep missingness as its own mask, and drop unobserved persons from numerator and denominator alike. An empty observed denominator now returns `None`, which the row assembly already carries through to JSON `null`; a bare `NaN` could not, because `write_reform_validation` serializes with `allow_nan=False`. `_person_rate` and `_level_total` become `-> float | None`. This is numerically inert on the pinned engine. `in_poverty` is `value_type = bool` on policyengine-us 2.2.1, so nothing is missing, `observed` is all-True and `raw == 1` equals `raw > 0`. The change lands ahead of any engine bump so the bump cannot silently move the 104 published state SPM rates. Tests: a `{0, 1, NaN}` indicator gives poor ÷ observed with both pre-fix answers as negative controls; an all-`NaN` slice serializes as `null`; a bare `NaN` refuses at the writer; and a Boolean indicator reproduces the pre-fix formula, recomputed in the test rather than transcribed. Co-Authored-By: Claude Opus 5 --- changelog.d/us-person-rate-nullable.fixed.md | 1 + .../build/us_runtime/reform_validation.py | 29 ++- .../tests/test_reform_validation.py | 193 ++++++++++++++++++ 3 files changed, 216 insertions(+), 7 deletions(-) create mode 100644 changelog.d/us-person-rate-nullable.fixed.md diff --git a/changelog.d/us-person-rate-nullable.fixed.md b/changelog.d/us-person-rate-nullable.fixed.md new file mode 100644 index 0000000000..6257dead24 --- /dev/null +++ b/changelog.d/us-person-rate-nullable.fixed.md @@ -0,0 +1 @@ +Read the SPM poverty indicator as nullable in `reform_validation._person_rate`: divide poor by *observed* persons rather than by everyone, and publish an empty observed denominator as JSON `null` instead of a 0% rate. Numerically inert on the pinned engine, where `in_poverty` is Boolean. diff --git a/packages/microcosm-build/src/microcosm/build/us_runtime/reform_validation.py b/packages/microcosm-build/src/microcosm/build/us_runtime/reform_validation.py index 1cc941c0df..2ee2456daf 100644 --- a/packages/microcosm-build/src/microcosm/build/us_runtime/reform_validation.py +++ b/packages/microcosm-build/src/microcosm/build/us_runtime/reform_validation.py @@ -917,13 +917,21 @@ def _state_sliced_total(variable: str, state: str, period: int) -> float: (np.asarray(values)[mask] * np.asarray(values.weights)[mask]).sum() ) - def _person_rate(level: BaselineLevelSpec) -> float: - """Weighted share of persons with ``variable`` truthy among the mask - population, sliced to ``state`` when set. + def _person_rate(level: BaselineLevelSpec) -> float | None: + """Weighted share of persons measured as ``variable == 1`` among the + *observed* mask population, sliced to ``state`` when set. Everything is computed at person level: spm_unit/household variables broadcast down to persons, and the numeric ``state_fips`` broadcasts where the string ``state_code_str`` cannot. + + The indicator is read as nullable. A person whose indicator is + missing is neither poor nor non-poor: they leave the numerator *and* + the denominator, and a slice with no observed person yields ``None`` + (serialized as JSON ``null``), never ``0.0``. Under a Boolean + indicator -- ``in_poverty`` is ``value_type = bool`` on the pinned + engine -- nothing is missing, so this is numerically identical to the + previous ``> 0`` share. """ nonlocal baseline import numpy as np @@ -932,8 +940,14 @@ def _person_rate(level: BaselineLevelSpec) -> float: baseline = simulate(None) # type: ignore[misc] values = baseline.calculate(level.variable, level.period, map_to="person") weights = np.asarray(values.weights) - flags = np.asarray(values) > 0 - mask = np.ones(len(flags), dtype=bool) + # Do not cast the indicator to bool or select on it directly: ``NaN > + # 0`` is False, which would silently read an unmeasured person as not + # poor. Compare the observed indicator with 1, and keep missingness as + # its own mask. + raw = np.asarray(values).astype(float) + observed = ~np.isnan(raw) + flags = raw == 1 + mask = observed.copy() if level.mask_variable: mask &= ( np.asarray( @@ -950,10 +964,11 @@ def _person_rate(level: BaselineLevelSpec) -> float: mask &= fips == US_STATE_POSTAL_TO_NUMERIC_FIPS[level.state] denominator = float(weights[mask].sum()) if denominator == 0: - return 0.0 + # No observed person in the slice: the rate is missing, not zero. + return None return float((flags[mask] * weights[mask]).sum() / denominator) - def _level_total(level: BaselineLevelSpec) -> float: + def _level_total(level: BaselineLevelSpec) -> float | None: nonlocal baseline if level.statistic == "rate": return _person_rate(level) diff --git a/packages/microcosm-build/tests/test_reform_validation.py b/packages/microcosm-build/tests/test_reform_validation.py index 263b54f916..efb80477f1 100644 --- a/packages/microcosm-build/tests/test_reform_validation.py +++ b/packages/microcosm-build/tests/test_reform_validation.py @@ -1247,6 +1247,199 @@ def spec(id_, *, state=None, mask=None, value=0.5): assert row["in_sample"] is False +def _nullable_rate_spec(id_, *, state=None, mask=None): + from microcosm.build.us_runtime.reform_validation import BaselineLevelSpec + + return BaselineLevelSpec( + id=id_, + name=id_, + variable="in_poverty", + period=2024, + benchmark_value=0.5, + benchmark_year="2022-2024", + source="Census", + source_url="https://census.gov/", + statistic="rate", + state=state, + mask_variable=mask, + ) + + +def test_rate_level_divides_poor_by_observed_under_a_nullable_indicator(): + """A missing indicator leaves both the numerator and the denominator. + + Reading ``NaN > 0`` as "not poor" -- what the pre-fix path did -- keeps the + unmeasured person in the denominator and deflates the published rate. The + negative controls below are the two wrong answers that arithmetic gives. + """ + import numpy as np + + # Five persons, one state. Person 2 and person 4 are unmeasured. + poverty = np.array([1.0, 0.0, np.nan, 1.0, np.nan]) + sim = _FakePersonSim( + { + "in_poverty": poverty, + "is_child": [1, 1, 1, 0, 1], + "state_fips": [1, 1, 1, 1, 1], + }, + weights=[10.0, 30.0, 50.0, 20.0, 40.0], + ) + + payload = reform_validation_payload( + [], + period=2024, + simulate=lambda reform: sim, + baseline_levels=[ + _nullable_rate_spec("overall"), + _nullable_rate_spec("al_overall", state="AL"), + _nullable_rate_spec("child", mask="is_child"), + ], + ) + rows = {r["id"]: r for r in payload["reforms"]} + + # Poor weight 10 + 20 = 30 over observed weight 10 + 30 + 20 = 60. + assert rows["overall"]["microcosm"]["budget_effect"] == pytest.approx(0.5) + # Negative control: poor over *all* persons, the pre-fix answer. + assert rows["overall"]["microcosm"]["budget_effect"] != pytest.approx(30.0 / 150.0) + # The state slice is the same population here, so it must agree. + assert rows["al_overall"]["microcosm"]["budget_effect"] == pytest.approx(0.5) + # Children observed: persons 0 and 1 (weights 10 + 30); poor among them 10. + assert rows["child"]["microcosm"]["budget_effect"] == pytest.approx(0.25) + # Negative control: the pre-fix child answer, 10 / (10 + 30 + 50 + 40). + assert rows["child"]["microcosm"]["budget_effect"] != pytest.approx(10.0 / 130.0) + + +def test_fully_unobserved_rate_slice_serializes_as_json_null(tmp_path): + """An empty observed denominator is a missing rate, not a 0% poverty rate. + + Before the fix this slice published ``0.0``: every ``NaN > 0`` flag was + False while the all-True mask kept the denominator positive, so the + ``denominator == 0`` branch was never reached. The failure was a + plausible-looking number, not an error. + """ + import numpy as np + + # AL (fips 1) is wholly unmeasured; CA (fips 6) is wholly observed. + sim = _FakePersonSim( + { + "in_poverty": np.array([np.nan, np.nan, 1.0, 0.0]), + "is_child": [1, 1, 1, 1], + "state_fips": [1, 1, 6, 6], + }, + weights=[10.0, 30.0, 20.0, 20.0], + ) + + payload = reform_validation_payload( + [], + period=2024, + simulate=lambda reform: sim, + baseline_levels=[ + _nullable_rate_spec("al_overall", state="AL"), + _nullable_rate_spec("ca_overall", state="CA"), + ], + ) + rows = {r["id"]: r for r in payload["reforms"]} + assert rows["al_overall"]["microcosm"]["budget_effect"] is None + assert rows["al_overall"]["microcosm"]["baseline_total"] is None + # Negative control: the observed slice still publishes a real rate. + assert rows["ca_overall"]["microcosm"]["budget_effect"] == pytest.approx(0.5) + + path = write_reform_validation(payload, tmp_path / "reform_validation.json") + raw = path.read_text() + assert "NaN" not in raw + written = json.loads(raw) + by_id = {r["id"]: r for r in written["reforms"]} + assert by_id["al_overall"]["microcosm"]["budget_effect"] is None + assert by_id["ca_overall"]["microcosm"]["budget_effect"] == pytest.approx(0.5) + + +def test_write_reform_validation_refuses_a_bare_nan_rate(tmp_path): + """Why the fix returns ``None`` rather than ``NaN``. + + ``write_reform_validation`` serializes with ``allow_nan=False``, so a bare + ``NaN`` reaching a row refuses instead of publishing. ``None`` is the only + value that both refuses to look like a rate and survives the writer. + """ + payload = { + "schema_version": REFORM_VALIDATION_SCHEMA_VERSION, + "baseline_period": 2024, + "scoring_window": "see per-reform jct.window", + "out_of_sample_simulated": True, + "reforms": [{"id": "x", "microcosm": {"budget_effect": float("nan")}}], + } + with pytest.raises(ValueError): + write_reform_validation(payload, tmp_path / "reform_validation.json") + + +def test_rate_level_is_inert_on_a_boolean_indicator(): + """Regression: on today's engine ``in_poverty`` is ``value_type = bool``. + + With no missing cell, ``observed`` is all-True and ``raw == 1`` equals the + old ``raw > 0``, so every published rate must be bit-for-bit what the + pre-fix path produced. The expected values are recomputed here from the + old formula rather than transcribed. + """ + import numpy as np + + poverty = np.array([True, False, True, True]) + assert poverty.dtype == np.dtype(bool) + weights = np.array([10.0, 30.0, 20.0, 20.0]) + child = np.array([1, 1, 0, 1]) + fips = np.array([1, 1, 6, 6]) + sim = _FakePersonSim( + {"in_poverty": poverty, "is_child": child, "state_fips": fips}, + weights=weights, + ) + + def legacy_rate(state=None, mask=None): + """The pre-fix aggregation, verbatim: ``> 0`` over an all-True mask.""" + flags = np.asarray(poverty) > 0 + keep = np.ones(len(flags), dtype=bool) + if mask is not None: + keep &= np.asarray(mask) > 0 + if state is not None: + keep &= fips == state + denominator = float(weights[keep].sum()) + if denominator == 0: + return 0.0 + return float((flags[keep] * weights[keep]).sum() / denominator) + + payload = reform_validation_payload( + [], + period=2024, + simulate=lambda reform: sim, + baseline_levels=[ + _nullable_rate_spec("us_overall"), + _nullable_rate_spec("al_overall", state="AL"), + _nullable_rate_spec("ca_overall", state="CA"), + _nullable_rate_spec("us_child", mask="is_child"), + _nullable_rate_spec("al_child", state="AL", mask="is_child"), + ], + ) + rows = {r["id"]: r for r in payload["reforms"]} + assert rows["us_overall"]["microcosm"]["budget_effect"] == pytest.approx( + legacy_rate() + ) + assert rows["al_overall"]["microcosm"]["budget_effect"] == pytest.approx( + legacy_rate(state=1) + ) + assert rows["ca_overall"]["microcosm"]["budget_effect"] == pytest.approx( + legacy_rate(state=6) + ) + assert rows["us_child"]["microcosm"]["budget_effect"] == pytest.approx( + legacy_rate(mask=child) + ) + assert rows["al_child"]["microcosm"]["budget_effect"] == pytest.approx( + legacy_rate(state=1, mask=child) + ) + # Pin the numbers too, so a shared bug in both formulas cannot pass. + assert legacy_rate() == pytest.approx(50.0 / 80.0) + assert legacy_rate(state=1) == pytest.approx(0.25) + assert legacy_rate(state=6) == pytest.approx(1.0) + assert legacy_rate(mask=child) == pytest.approx(0.5) + assert legacy_rate(state=1, mask=child) == pytest.approx(0.25) + + def test_rate_spec_validation_errors(): from microcosm.build.us_runtime.reform_validation import BaselineLevelSpec