Generalize the ACA-branded labels in the shared binary assignment ops#338
Open
daphnehanse11 wants to merge 1 commit into
Open
Generalize the ACA-branded labels in the shared binary assignment ops#338daphnehanse11 wants to merge 1 commit into
daphnehanse11 wants to merge 1 commit into
Conversation
The generic assign_binary_from_rate / calibrate_binary_assignment / calibrate_binary_assignment_joint_targets handlers hard-coded "ACA binary assignment" / "ACA binary calibration" in every error message, so a failure in any non-ACA stage driving them (e.g. PR #334's person-grain medicaid_take_up stage) reported itself as an ACA failure. Labels are now keyed to the operation kind ("US binary assignment" / "US binary calibration" / "US joint binary calibration", matching the file's existing "US ratio computation" style) and threaded through the shared helpers so the joint handler reports its own name. The "aca:{output}" draw salt is deliberately unchanged (now commented as frozen): existing ACA releases are bit-reproducible against it, and label-only changes leave every seeded outcome byte-identical. The calibrate handlers' silent 'stable_tax_unit_draw' draw-column default is removed: 'draw' is now a required parameter (the assign handler already required it), and the ACA manifest states the previous default explicitly, so behavior is unchanged while a future person-grain stage that omits the param fails loudly instead of minting positionally-keyed draws under a tax-unit column name. _stable_draws documents its tax_unit_id-vs-positional keying fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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
The generic
assign_binary_from_rate/calibrate_binary_assignment/calibrate_binary_assignment_joint_targetshandlers inus_runtime/source_runtime.pyhard-coded "ACA binary assignment" / "ACA binary calibration" into every error message, even though they are shared manifest primitives. A failure in any non-ACA stage driving them — empirically verified with #334's person-grainmedicaid_take_upstage, where droppingperson_weightyielded "ACA binary calibration frame is missing required column(s)" — reported itself as an ACA failure. Closes the follow-up from the #334 review._binary_calibration_value_state,_preserved_true_mask,_first_available_target_table,_group_columns,_domain_mask) take alabelkeyword so each handler reports its own name. The_ACA_prefix is dropped from the corresponding parameter-key frozensets.aggregate_person_to_tax_unitkeeps its ACA labels — it is genuinely ACA-specific (Marketplace coverage aggregation) and only the ACA stage uses it.aca:{output},calibrate:{variable}, andjoint-calibrate:{variable}are byte-identical, and theaca:prefix is now commented as frozen: existing ACA releases are bit-reproducible against it. The exact-seeded-outcome assertions in the existing ACA tests pass unchanged, which locks this in.stable_tax_unit_drawdefault is removed:drawis now a required parameter (the assign handler already required it), and the ACA manifest's three calibrate ops state the previous default explicitly — bit-identical behavior. A future person-grain stage that omits the param now fails loudly instead of silently minting positionally-keyed (not identity-stable) draws under a tax-unit column name._stable_drawsgained a docstring stating itstax_unit_id-vs-positional-index keying and that stages at other grains should mint their own identity-keyed draw column.Interaction with open PR #334 (medicaid_take_up)
medicaid_take_upstage tosource_stages.json; this PR edits only the existing ACA stage's calibrate ops. The hunks are disjoint, so the two merge cleanly in either order.draw: "stable_person_draw"param and pre-compute the draw column identity-keyed at person grain, so they comply with the stricter required-drawcontract as written.Tests
drawerrors for both calibrate handlers, and a person-grain calibration happy path with a declared draw column.packages/populace-buildsuite passes (884 tests); ruff check/format clean on the changed files.🤖 Generated with Claude Code