Conversation
|
Hi @juaristi22, could you please review this PR when you have a chance? This is my first time working in a data repo, and I’m not yet confident that my approach to adding the childcare attendance inputs follows Microcosm’s requirements and conventions. Please be as critical and thorough as you would normally be—don’t hold back because it’s my first contribution here. I’d especially appreciate feedback on the survey mapping, imputation approach, build integration, and whether the validation is sufficient. Please also point out even minor issues with wording, naming, formatting, or code organization; I want to learn the repo’s standards and get this right. The PR description includes a “Survey data for reviewers” section with the two source files and a README explaining how they are used. The Drive folder is accessible with a signed-in PolicyEngine Google account. The remaining limitations and the separate integration needed for #893 are documented as well. Thank you! |
Program reviewBase repository: PolicyEngine/microcosm Source DocumentsNo source documents registered; see scope and validation. What looks good
CriticalC1 — CRITICAL (Must Fix): attendance values are not bound to their source receipt, and refreshing an enriched base silently retains stale values (OPEN)Location: Trigger / reproduction: use an otherwise valid base H5 containing non-default values for the three attendance columns (the native candidate produced by this PR is such an H5) and run the fiscal builder without the two NSECE TSV flags. The argument parser only checks that the flags are paired when one is supplied, the stage is skipped when both are absent, the generic H5 loader reads only the six entity tables and discards Expected: a release must either execute the pinned source stage or validate and carry a receipt cryptographically/content-bound to the exact persisted attendance values. Re-running with a changed source identity or seed must either recompute derived cells or reject the incompatible pre-existing provenance. Observed: arbitrary or stale non-default values satisfy the hard coverage manifest, while source coverage may contain no attendance receipt; when the stage is requested on an enriched input, output values and new metadata can describe different executions. Impact: this defeats the repository's load-bearing artifact/provenance contract and can certify materially different state childcare-subsidy outputs as if they came from the reviewed NSECE mapping. The committed comparison shows the attendance inputs move potential modeled benefits from about $2.25B to $5.29B, so accepting unbound/stale values is output-material. The new tests cover flag pairing and same-input idempotence, but not receipt-required loading, changed-source/seed refresh, or final source-coverage enforcement. Should AddressA1 — SHOULD ADDRESS: the final release gate does not reassert row-complete attendance (OPEN)
A2 — SHOULD ADDRESS: invalid or missing household source identities collapse into one sibling-dependence group (OPEN)Location: Trigger / reproduction: pass a US frame whose Expected: source household identities used to couple sibling draws should be complete and every person link should resolve; invalid identity must fail closed, as comparable source-ID mapping code in Observed: unrelated children with unresolved source identities are treated as one synthetic household for the shared-rank component. Impact: on malformed or legacy inputs this introduces artificial cross-household dependence and masks an upstream linkage defect. The qualified BuildP parent likely satisfies the late-producer finite-ID invariant, so this does not refute the committed candidate, but the new public stage itself does not enforce its stated identity precondition. A3 — SHOULD ADDRESS: sibling validation covers only binary participation, while the shared rank couples full schedule intensity (OPEN)
SuggestionsS1 — SUGGESTION: foreground the questionnaire-transport discrepancy and define acceptance criteria (OPEN)
S2 — SUGGESTION: the persisted operation order collapses three distinct transformations to one generic label (OPEN)
Coordinator assessment: The code reviewer independently identified the same receipt-auditability issue; it is consolidated here once. Evidence Gaps
Notes
Validation SummaryInspected the five-commit, 36-file merge-base diff and affected runtime, build, serializer, coverage, documentation, tests, and aggregate evidence. Local tests: NOT RUN (environment unavailable; no dependency install). GitHub CI: 23/23 SUCCESS at head 3fe3e68. Official public source review covered the NSECE study page, Census ASEC variable catalog, and BLS CPI-U table; licensed source bytes/codebooks and full-data artifacts were unavailable. Timingsetup seconds: 29.00s; scope seconds: 67.00s; parallel review seconds: 765.00s; policy role seconds: 765.00s; code role seconds: 525.00s; adjudication seconds: 0.00s; consolidation cleanup seconds: 140.00s; elapsed seconds: 976.00s Review SeverityREQUEST_CHANGES. Open findings: 1 critical, 3 should address, 2 suggestions. |
Missing child-level attendance inputs can leave CCDF subsidies at zero. This PR adds dataset-side attendance from the real 2024 NSECE household/calendar files, integrates the complete stage into the US fiscal refresh builder, and qualifies it on the pinned BuildP population. PolicyEngine-US defaults remain unchanged.
Implementation
--production-stageexercises the same function used by the fiscal builder.Real population evidence
Reproduction and aggregate reports pin the source, parent, candidate, code, and engine identities. No survey records, donor microdata, or population artifacts are committed.
These are attendance-only counterfactuals using 2026 policies on fixed source ages/incomes, without aging or uprating. Modeled potential benefits are not calibrated spending or caseload estimates. The diagnostics informed development and are not an untouched external acceptance sample.
Scope and remaining limits
The 10,506 donors comprise 7,460 observed calendars and 3,046 reconstructed schedules; 1,105 under-13 source records remain excluded. Conditional matching cannot identify missing schedules for excluded children without assumptions. The May/fall bridge does not observe actual summer attendance or provider-specific pricing.
The explicit export policy retains the existing engine baseline for missing values outside ages 0–12 and labels that provenance. It does not establish nonattendance for older children, including 557 disabled teenagers in the target population. Observed values are preserved, and unresolved under-13 values fail export.
This implements and validates the attendance build stage; it does not publish a replacement population or claim that full population release certification is complete. Reports retain
production_ready: false. Licensed sources must be supplied to a release build, and its normal calibration/publication gates still apply.Survey data for reviewers
Download the NSECE source files used by this PR (Google Drive).
The folder contains NSECE-2024-PR916-source-files.zip (about 5.6 MB compressed; 392 MB extracted):
These are the two unchanged NSECE source files verified against the hashes in the committed validation report. The ZIP does not include CPS ASEC files or the target population. Access: sign in with a PolicyEngine Google account to view and download the ZIP; the folder is not publicly accessible.
Validation
Addresses #915; provider/activity/older-child gaps remain documented. PolicyEngine/policyengine-us#9405 concerns household aggregation and remains separate.