Fix the RCT power calculator, which was wrong in all three solve modes - #6
Merged
Merged
Conversation
Var(mean_treat - mean_control) = sigma^2/n_t + sigma^2/n_c. That factor of two
was missing from every per-arm expression in the power calculator, so every
answer erred in the direction that flatters a study:
solve mode reported correct
n per arm, d=0.2, 80% power 197 393
power at 393 per arm 97.8% 80.0%
MDE at 393 per arm 0.141 SD 0.200 SD
Simulated against a two-sample t-test at 40,000 replications, the old
recommendation of 197 per arm delivers 51% power, not 80%.
`n_total` was correct throughout, because 1/(p*(1-p)) equals 4 at p=0.5 and
already carries the two. That is why the error survived: the one output anyone
would check against a published table was the one that was right.
The file also mixed conventions. The allocation panel used total N and was
correct; the other ten call sites used per-arm and were not.
- Move the arithmetic to `python-shiny/rct-power-calculator/power.py`, which
imports no Shiny and is therefore testable. One convention throughout: N is
the total sample, split p and 1-p, with per-arm figures derived from it.
- Rewire all eleven call sites in `app.py`, including the four sensitivity
panels, which carried the same error.
- Report the treatment and control counts separately, since "per arm" is not
well defined once the allocation is unequal.
- Document the normal-approximation shortfall rather than hiding it: 63 per arm
at d=0.5 where G*Power asks for 64, simulating at 79.6% against a nominal 80%.
A test pins the size of the gap.
CI could not fail. The entire job was:
python -m py_compile python-shiny/*/app.py 2>/dev/null || echo "Syntax check complete"
The `|| echo` means a syntax error printed "Syntax check complete" and exited 0,
and the five R apps (5,113 lines) were not checked at all. CI now compiles every
Python app under `set -euo pipefail`, imports each one and asserts it defines a
Shiny App, runs the test suite, and parses every R app.
Also checked and found correct, so left alone: the Alkire-Foster implementation
in mpi-explorer (M0 = H x A with censored headcount ratios), and the NPV,
benefit-cost ratio, IRR, growth and discounted-payback functions in
cost-benefit-analysis, which agree with closed form exactly.
Adds CLAUDE.md, tests/ (19 tests against closed form and simulation) and
requirements-dev.txt.
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
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.
Six months since the last commit. The finding is serious enough to lead with.
Every solve mode was wrong, and all three flattered the study
Var(x̄_treat − x̄_control) = σ²/n_t + σ²/n_c. That factor of two was missing from every per-arm expression.Simulated against a two-sample t-test at 40,000 replications, 197 per arm delivers 51% power. Anyone who sized a trial with this tool under-recruited by half.
Why it survived.
n_totalwas correct throughout, because1/(p(1−p))equals 4 at p = 0.5 and already carries the two. The one output anyone would sanity-check against a published table was the one that was right.The file also mixed conventions: the allocation sensitivity panel used total N and was correct, while the other ten call sites used per-arm and were not. That inconsistency is how it stayed hidden.
CI could not fail
The entire job was:
The
|| echomeans a syntax error prints "Syntax check complete" and exits 0. The2>/dev/nulldiscards the reason. And the five R apps (5,113 lines) were not checked at all.CI now compiles every Python app under
set -euo pipefail, imports each one and asserts it defines a ShinyApp(a file can compile and still fail at import), runs the test suite, and parses every R app.What changed
python-shiny/rct-power-calculator/power.py, which imports no Shiny and is therefore testable. One convention throughout: N is the total sample, splitpand1−p, with per-arm figures derived from it.app.pyrewired, including the four sensitivity panels.Tests
19 tests. Closed-form round trips (power and sample size are inverses; MDE and power are inverses; unequal allocation always costs sample; clustering costs exactly the design effect), plus simulation — the only check that would have caught the original defect.
One test deliberately pins the old error so a regression is recognisable: it asserts the previous expression gives 197 and that 197 simulates below 60% power.
A limitation stated rather than buried
The module uses the normal approximation, not the t distribution, so it understates the required sample slightly at small n: 63 per arm at d = 0.5 where G*Power asks for 64, and 25 at d = 0.8 where G*Power asks for 26. Simulated, those land at 79.6% and 79.1% against a nominal 80%. The gap is under one percentage point above roughly 25 per arm and widens below it.
test_normal_approximation_shortfall_stays_smallpins its size, and the README says so.Checked and found correct
Not everything was broken, and I am not going to invent findings. Verified against closed form and left alone:
mpi-explorer— Alkire-Foster is correct. M0 = H × A, and the censored headcount ratio is(dep_matrix[:, j] * is_poor).sum() / n, which is right.cost-benefit-analysis— NPV, BCR, IRR, geometric growth and discounted payback all agree with closed form exactly. NPV at 5% on a 1000/400×4 stream is 418.3802 both ways; the IRR of 21.8623% zeroes the NPV to six decimals.The five R apps now have a parse check and still have no test coverage.
gini-lorenzandpoverty-line-analysisboth compute quantities with closed forms worth asserting; that work is not done here andCLAUDE.mdsays so.Generated by Claude Code