[xidongdxi]

Fixes #47. Covers every thread in the issue (body + both comments).

1. Boundary marginal power 0/1 (#47 (comment))

When power_marginal contains 0 or 1, the simulated p-values are deterministically 1 or 0. A p-value of exactly 0 combined with a closure weight of 0 makes p <= weight * alpha evaluate to TRUE for a hypothesis that is not in the intersection, breaking two fast power paths for the two-hypothesis Holm graph with power_marginal = c(1, 0):

• graph_test_shortcut_fast() (Bonferroni) looped forever — an already-rejected hypothesis kept re-appearing in rejected_step, so any(rejected_step) stayed TRUE while rejected never changed.
• graph_test_closure_fast() (Hochberg/Simes/parametric) over-rejected — matrixStats::rowMaxs() reduced over hypotheses before masking by intersection membership, returning local power (1, 1) instead of (1, 0).

Fix: restrict rejections to hypotheses actually in the current intersection — a !rejected guard in the shortcut (which also terminates the loop) and masking rej_hyps by matrix_intersections before the row reduction in the closure. Only the degenerate p == 0 & weight == 0 boundary changes; with p > 0 the comparisons are identical. The fast power paths now agree with the exact graph_test_shortcut() / graph_test_closure().

input	before	after
Bonferroni c(1, 0)	hangs	(1, 0)
Hochberg c(1, 0)	(1, 1)	(1, 0)
c(0, 1), c(1, 1), c(0, 0)	—	exact

2. alpha of exactly 0 or 1 (@EeethB's comment)

power_marginal is the marginal power at level alpha, so the noncentrality mapping qnorm(1 - alpha) - qnorm(1 - power_marginal) is only defined for alpha strictly inside (0, 1). At 0 or 1 it produced:

• NaN → "missing value where TRUE/FALSE needed" (e.g. alpha = 0, power_marginal = c(0, 0)), or
• silently wrong power (e.g. alpha = 0 rejected every hypothesis — (1, 1) — and alpha = 1 rejected none).

Fix: power_input_val() now requires alpha strictly between 0 and 1, with a clear message. Interior alpha with boundary marginal power (0 or 1) remains valid and is handled by item 1. This is the "null check" @EeethB suggested.

3. force_closure discrepancy (issue body)

The original report compared force_closure = TRUE vs the default for marginal_power = c(0, 0, 0). Those arguments (force_closure, sim_seed) no longer exist in the current API — graph_calculate_power() now picks the shortcut path for all-Bonferroni and the closure path otherwise, internally. With item 1 fixed, both internal paths agree with the exact closed test on the boundary, so this discrepancy is resolved. No separate change needed.

On "should validation exclude 0/1 for marginal power?"

No. power_marginal of 0 or 1 is a legitimate boundary (never / always reject) and now computes correctly. Only alpha of 0/1 is excluded, since it is genuinely undefined for a power calculation.

Tests

Added regression tests in test-graph_calculate_power.R: marginal power 0/1 for Bonferroni and Hochberg (incl. agreement with the exact closed test), and the four alpha 0/1 cases. Full suite passes (560 passed, 0 failed, 13 skipped for optional igraph/gMCP).

🤖 Generated with Claude Code

[github-actions]

Code Coverage Summary

Filename                              Stmts    Miss  Cover    Missing
----------------------------------  -------  ------  -------  -----------------------------------------------
R/adjust_p.R                             44       1  97.73%   173
R/adjust_weights_parametric_util.R       55       0  100.00%
R/adjust_weights.R                       49       0  100.00%
R/as_graph.R                             39       2  94.87%   90, 107
R/edge_pairs.R                           11       1  90.91%   31
R/example_graphs.R                      181      15  91.71%   304, 408-422
R/graph_calculate_power.R               175       0  100.00%
R/graph_create.R                         75       0  100.00%
R/graph_generate_weights.R               45       0  100.00%
R/graph_rejection_orderings.R            35       0  100.00%
R/graph_test_closure.R                  181      10  94.48%   204-205, 300, 383-388, 401
R/graph_test_shortcut_gsd.R             375      13  96.53%   248, 298-302, 304, 306, 629, 676, 740, 802, 821
R/graph_test_shortcut.R                 106       0  100.00%
R/graph_update.R                         62       0  100.00%
R/gs_boundaries.R                        56       8  85.71%   60-61, 116-117, 120-121, 126-127
R/gs_corr.R                               4       0  100.00%
R/plot.initial_graph.R                   67       9  86.57%   137, 157, 163, 184-187, 204-207
R/plot.updated_graph.R                    4       0  100.00%
R/power_tests.R                          21       0  100.00%
R/print.graph_report.R                  177       1  99.44%   171
R/print.gsd_graph_report.R              156       4  97.44%   95-96, 98, 189
R/print.initial_graph.R                  39       0  100.00%
R/print.power_report.R                  138       0  100.00%
R/print.updated_graph.R                  43       0  100.00%
R/repeated_p.R                           65       0  100.00%
R/sequential_p.R                         64       0  100.00%
R/spending_functions.R                  106       9  91.51%   145, 270-275, 433-434
R/test_graph_names.R                     25       0  100.00%
R/test_power_input_val.R                124       0  100.00%
R/test_values.R                          80       0  100.00%
TOTAL                                  2602      73  97.19%

Diff against main

Filename                     Stmts    Miss  Cover
-------------------------  -------  ------  --------
R/graph_calculate_power.R        0      -6  +3.43%
R/power_tests.R                 +1       0  +100.00%
R/test_power_input_val.R        +2       0  +100.00%
TOTAL                           +3      -6  +0.23%

Results for commit: ec74a47

Minimum allowed coverage is 0%

♻️ This comment has been updated with latest results