Skip to content

Fix group-sequential threshold misalignment for hypotheses first tested after analysis 1#2

Open
lhunt13 wants to merge 1 commit into
johnsonandjohnson:mainfrom
lhunt13:fix/threshold-alignment-analyses-analysed
Open

Fix group-sequential threshold misalignment for hypotheses first tested after analysis 1#2
lhunt13 wants to merge 1 commit into
johnsonandjohnson:mainfrom
lhunt13:fix/threshold-alignment-analyses-analysed

Conversation

@lhunt13

@lhunt13 lhunt13 commented Jul 13, 2026

Copy link
Copy Markdown

Problem

In the simulation-based operating characteristics, update_p_thresholds_cpp writes each hypothesis's per-look rejection boundaries into the threshold matrix starting at analysis column 1, while get_maurer_bretz_z_raw_cpp places the observed p-values at the hypothesis's analyses_analysed columns. The two are consistent only when a hypothesis's first tested analysis is analysis 1.

For any multi-look (alpha-spending) hypothesis first tested later — e.g. an OS endpoint tested at analyses 2 & 3 — the stringent interim boundary lands on an analysis where the hypothesis is never tested (and is silently discarded), while its real interim look is tested against the final-look boundary. Simulated unconditional power (Tables 6a/6b) is inflated above the hypothesis's full-alpha local power (an upper bound), and the local test over-spends alpha (e.g. cumulative Type-I error 0.033 vs the 0.025 target at full weight).

Design boundaries, nominal p-values, and local power (Tables 1/4/5) are computed correctly; only the simulated operating characteristics and quantities derived from them (expected success analysis/time, subset powers) are affected.

This also fires in the bundled platform_study example. Re-running it patched vs unpatched (same seed), Table 6a changes only for the staggered multi-look hypotheses — power at their interim (analysis 2) drops by up to 10.5 percentage points (H8/H9: 0.657 → 0.552; H10/H11: 0.266 → 0.227; H3–H7: 0.913 → 0.894) — while hypotheses first tested at analysis 1 (H1, H2) and the single-look H12 are bit-for-bit unchanged, exactly matching the bug's scope.

Minimal demonstration

Two asOF hypotheses, H1 tested at analyses 1 & 2, H2 (OS) at analyses 2 & 3, all alpha starting on H1:

M <- update_p_thresholds_cpp(d$analyses, hyp, cfg$graph$g, c(0, 1))  # H2 holds full alpha
# H2 per-look OBF boundaries (looks at analyses 2 & 3): 0.01007, 0.02201
# Before fix, H2 row of M (columns = analyses 1..3):    0.01007, 0.02201, 0.02201
# After fix:                                            -1,      0.01007, 0.02201

Before the fix, H2's simulated unconditional power at its interim (57.6%) exceeded its full-alpha local power there (47.2%), which is impossible for a valid alpha-spending boundary. After the fix it is 45.3% / 70.6% — properly bounded by the 47.2% / 72.7% local power.

Fix

update_p_thresholds_cpp now indexes thresholds by analyses_analysed, mirroring the p-value placement in get_maurer_bretz_z_raw_cpp:

  • Each per-look boundary is written at its analyses_analysed column.
  • Thresholds are then forward-filled across later analyses. This deliberately mirrors the existing forward-fill of p_obs, so a hypothesis retested at a later global analysis (after weight propagation) is tested against its most recent look's boundary rather than an unrelated one.
  • Columns before a hypothesis's first look keep the -1 sentinel; p_obs is NA there, so the hypothesis can never be spuriously rejected at analyses preceding its first look.

Behavior is unchanged for hypotheses whose looks start at analysis 1, and for single-look hypotheses.

Tests

New tests/testthat/test-sim-thresholds.R:

  1. Exact placement — for a staggered design (analyses_analysed = 2:3), the interim/final boundaries land at analysis columns 2/3, column 1 stays at the -1 sentinel, and a hypothesis starting at analysis 1 keeps its previous placement including the carried-forward final boundary.
  2. Power invariant — simulated unconditional power of the staggered hypothesis does not exceed its full-alpha local power (within Monte-Carlo tolerance at 4,000 sims; the pre-fix violation was ~10 percentage points, well outside tolerance).

Both tests fail on main and pass with this change. Full testthat suite passes (0 failures, 106 passing); R CMD check is clean apart from the two notes already present on main (gsDesign2::: imports and the foreign .C("gsbound1") call). NEWS.md updated.

Possibly-separate observation (not addressed here)

After this fix, platform_study still reports simulated power above the full-alpha analytic local power for some OS hypotheses (e.g. H1: 0.744 vs 0.723 at its first look; H8: 0.552 vs 0.350). This is unrelated to the threshold alignment: it also affects H1, whose looks start at analysis 1 and whose Table 6a values are identical before and after this patch. It may be a design-vs-simulation information mismatch specific to OS endpoints in that example, or expected behavior of the platform design; happy to investigate and file separately.

🤖 Generated with Claude Code

…ed after analysis 1

In update_p_thresholds_cpp, per-look rejection boundaries were written into
the threshold matrix starting at analysis column 1, while
get_maurer_bretz_z_raw_cpp places observed p-values at the hypothesis's
analyses_analysed columns. For any multi-look hypothesis whose first tested
analysis is not analysis 1 (e.g. OS tested at analyses 2 & 3), the stringent
interim boundary landed on an analysis where the hypothesis is never tested
and was silently discarded, while the real interim look was tested against
the final-look boundary. This inflated simulated unconditional power
(Tables 6a/6b) above the hypothesis's full-alpha local power and over-spent
alpha; design boundaries and local power (Tables 1/4/5) were unaffected.

Thresholds are now indexed by analyses_analysed, mirroring the p-value
placement, and forward-filled across later analyses to match the existing
forward-fill of observed p-values (so a hypothesis retested after weight
propagation uses its most recent look's boundary). Columns before a
hypothesis's first look keep the -1 sentinel, matching NA p-values there.

Adds regression tests: exact boundary placement for a staggered
(analyses_analysed = 2:3) hypothesis, and the invariant that simulated
unconditional power cannot exceed full-alpha local power.

Co-Authored-By: Claude Fable 5 <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