Fix size-less RNG draws that made 15 columns constant; add tests and an answer key - #9
Merged
Merged
Conversation
…an answer key
Twenty-two call sites across ten generators called numpy's RNG without a size
argument, so the draw returned a scalar that broadcast across the whole column:
owns_radio = rng.binomial(1, 0.55) # one draw, every row
months_displaced = np.where(displaced, rng.exponential(14), 0) # one draw, every row
Nothing raised, the row count was right and the file wrote. Fifteen columns were
constants, among them four asset variables in `targeting` (a proxy-means-test
dataset whose asset predictors did not vary) and the three dropout barriers in
`girls_education`, all identically zero. The neighbouring `barrier_cost` was
always fine because its probability is an array, which is what confirmed the
diagnosis.
None of it was visible because CI ran `python generate.py --list`, which imports
no generator. All 36 modules were unexecuted by any automated check.
Generators
- Add the missing size argument at all 22 sites. Scalar draws that are genuinely
parameters (per-arm compliance rate, per-country intercept, anything inside a
per-row loop) are left alone.
- rct_experiment: `spillover_risk` was 1 for every control and 0 for every
treated unit, an exact alias of the treatment dummy, because randomisation was
stratified within district so every district always held treated units. Add a
village level, randomise 70% of villages into treatment and 30% into pure
control, and give exposed controls a real +3% spillover. The flag now varies,
and using contaminated controls biases the ITT toward zero by ~0.011 log
points, which is the lesson the design should teach.
- public_health: rebuild PHQ-9 as a sum of nine 0-3 items. The previous
`9 * logistic(...) * 3` was bounded with no item-level variance, so the floor
sat at 2 and `depression_severe` was identically zero in every draw. Now spans
0-27 with 14.6% at or above the moderate threshold and 1.45% at severe.
- girls_education: schooling status is three-state. `dropped_out` was defined as
the complement of `enrolled`, which both collapsed never-enrolled girls into
dropouts and made the two columns perfect aliases. Also vectorise the distance
loop and remove a dead `2 if True else 5` conditional.
Tests and documentation
- tests/: 233 tests. Every generator runs, is reproducible under a seed, and
responds to a seed change; the registry matches the modules on disk; no column
is constant or a perfect alias of another, with the legitimate exceptions
(IRT item parameters, the poverty line) listed with reasons.
- TRUTH.md: the answer key. Records which parameters are in the data and which
estimand recovers each. The ITT is not a stable target because take-up is
drawn U(0.65, 0.85) per run and moved between +0.089 and +0.203 across five
seeds; the Wald LATE recovers theory to within 0.002.
- CLAUDE.md: new, so the next session does not re-derive this.
- CI: run the suite on 3.11 and 3.12, plus an end-to-end job writing every
dataset to CSV and Parquet, since serialisation fails differently.
- Pin requirements exactly. `pandas>=2.0` would pick up pandas 3, where text
columns carry a `str` dtype and a check keyed on `dtype == object` silently
skips every text column.
- README: correct the output directory (`output/`, not `data/`), the Python
floor (3.11), a dependency list naming `faker`, which nothing imports, and
claims that the data mirrors the distributions of real surveys.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MrvR2NXsJFVRCeJZFCPuNL
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
2 participants
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.
What does this PR do?
rng.binomial(1, 0.55)without a size argument returns a scalar, which numpy then broadcasts across the whole column. Nothing raises, the row count is right, the file writes, and the variable is a constant.It was at nineteen call sites across nine generators and produced fifteen constant columns — including four asset variables in
targeting, a proxy-means-test dataset whose asset predictors did not vary. A learner practising on it was fitting a model to columns that carry no information.It appears in two shapes, and the second is the dangerous one:
The second leaves the column holding two values, so it survives any "is this column constant" check.
tests/test_no_degenerate_columns.pytherefore tests for perfect binary aliasing as well as for constants, and runs against every registered generator automatically.The discriminator when reading code is whether the probability is a scalar or an array. A scalar draw is correct when it is a parameter rather than a column — the per-arm compliance rate, the per-country intercept — and those are deliberately left alone.
CI previously ran
python generate.py --listand nothing else, which imports no generator, so all thirty-six modules were unexecuted by any automated check. It now runs the suite on 3.11 and 3.12 plus an end-to-end job writing every dataset to CSV and Parquet, because serialisation is a separate failure surface: parquet is stricter about mixed types than csv.TRUTH.mdis the answer key a learner checks against, so a failing test means either the generator changed orTRUTH.mdis now lying. Do not loosen a tolerance to make one pass.Type of change
Checklist
python -m pytest tests/ -qpassesCommits
🤖 Generated with Claude Code
https://claude.ai/code/session_01MrvR2NXsJFVRCeJZFCPuNL
Generated by Claude Code