Skip to content

Give md_ccs_service_unit an explicit UNIT_1 select default - #9402

Open
MaxGhenis wants to merge 2 commits into
PolicyEngine:mainfrom
MaxGhenis:fix-md-ccs-service-unit-enum-default
Open

Give md_ccs_service_unit an explicit UNIT_1 select default#9402
MaxGhenis wants to merge 2 commits into
PolicyEngine:mainfrom
MaxGhenis:fix-md-ccs-service-unit-enum-default

Conversation

@MaxGhenis

Copy link
Copy Markdown
Contributor

Summary

Follow-up to PolicyEngine/policyengine-us#9400. That PR's sweep of Enum-valued variables found md_ccs_service_unit as the one remaining select whose conditions are not exhaustive. Its unit_hours bracket returns 0 units for negative or NaN childcare_hours_per_day (verified: calc([-1, 0, 2, 3, 3.0001, 6, 10, nan]) gives [0, 1, 1, 1, 2, 3, 3, 0]), and no condition matched 0, so numpy's select filled those rows with the integer 0.

What happens today

  • The raw formula output for a household with an adult (0 hours) and a child with -1 hours is the object array [UNIT_1, 0]. Encoding that directly raises AttributeError: 'int' object has no attribute 'index'; with the unmatched row first it raises ValueError: Invalid value(s) ['0', 'MDCCSServiceUnit.UNIT_1'] for enum MDCCSServiceUnit.
  • A full Simulation does not crash today. The variable has defined_for = StateCode.MD, and policyengine-core's defined_for path (simulation.py: np.where(mask, array, default_value) followed by item.index if isinstance(item, Enum) else item) passes the bare 0 through as Enum index 0, which happens to be UNIT_1. Setting defined_for = None on the loaded variable reproduces the crash. So the current answer is right by coincidence of member order; this PR makes it right by rule.

Fix

Drop the redundant units == 1 branch and give the select default=MDCCSServiceUnit.UNIT_1, mirroring #9400. UNIT_1 is the semantically right default: COMAR 13A.14.06.02B(59)(a) defines one unit as 3 hours or less per day, so anything at or below that ceiling, including out-of-range hours, is one unit. It also matches how zero hours is already treated. No output changes for any hours at or above 0 (units 1, 2, 3 map exactly as before); negative hours keep producing UNIT_1. The variable's reference now cites the definitions section (13A.14.06.02) alongside the payment section (.11).

Consumers are consistent with the zero-hours treatment: md_ccs_weekly_copay charges $1/week for UNIT_1 (eligible children only; eligibility does not depend on hours) and md_ccs_payment_rate scales the UNIT_3 base rate by 1/3.

Considered and not done: clamping with max_(hours, 0). It is redundant given the default for negative hours, and numpy.maximum propagates NaN, so it would not cover the NaN case anyway. NaN cannot be supplied as a direct input (core rejects it in set_input); the default guards the computed path regardless.

Tests

  • md_ccs_service_unit.yaml: negative hours alone; an adult listed first with a negative-hours child second (the ordering that raises AttributeError on the raw path); a negative-hours child first with a full-time sibling (the ValueError ordering).
  • edge_cases.yaml Case 38: negative hours give the $1/week copay, the same as Case 36 (zero hours).
  • md_ccs_payment_rate.yaml: negative hours in a Region X licensed center give $127/week ($381 UNIT_3 base × 1/3).
  • New policyengine_us/tests/test_md_ccs_service_unit_enum_default.py (collected by make test-other-python): encodes the raw formula output directly, which is the path a variable without defined_for takes, for both row orderings and for NaN. Two of its three tests fail on unfixed main (04a961a) with the object array [UNIT_1, 0]; all three pass with the fix.
  • The YAML cases also pass on unfixed main because of the defined_for passthrough, so they pin the intended behaviour rather than catch the crash; the pytest is the regression guard.
  • All 86 MD CCS YAML tests pass:
.venv/bin/policyengine-core test policyengine_us/tests/policy/baseline/gov/states/md/msde/ccs -c policyengine_us
  • uvx ruff@0.9.0 format and ruff check are clean on the changed Python files. Local environment: Python 3.14.4, policyengine-core 3.30.2.

Related

  • #9400 fixes the same bug class in ccdf_age_group and left this variable for a follow-up.

🤖 Generated with Claude Code

Negative or NaN childcare hours map to 0 units in the unit_hours bracket,
which no select condition matched, so numpy filled those rows with the
integer 0. A full simulation only avoided a crash because core's
defined_for path passes a bare integer through as Enum index 0, which
happens to be UNIT_1. Make UNIT_1 the explicit default per COMAR
13A.14.06.02B(59) (one unit is 3 hours or less per day), add YAML and
pytest coverage, and cite the definitions section. Follow-up to PolicyEngine#9400.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (e21799b) to head (a03e82e).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #9402   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         1    -2     
  Lines           33        19   -14     
=========================================
- Hits            33        19   -14     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Address two review nits. The comment now describes the UNIT_1 default
as a fallback for unmatched values (negative, NaN and infinite hours all
map to 0 units in the bracket) rather than values below the one-unit
ceiling. The NaN test seeds full-time hours and asserts the injected NaN
so a failed cache injection cannot pass by coincidence.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.

1 participant