From dd625de068ee0b8e61221f16af99e2e326ed1bf2 Mon Sep 17 00:00:00 2001 From: PJ Date: Tue, 18 Aug 2026 20:17:39 +0530 Subject: [PATCH] docs(analyze): name the tests the tool actually runs The --paired flag advertised the signed-rank, two comments and a test message still said rank-sum, and nothing said what rankSum is doing in the tree now that no campaign reaches it. --- cmd/internal-tools/analyze/analysis_test.go | 2 +- cmd/internal-tools/analyze/main.go | 2 +- cmd/internal-tools/analyze/planted_test.go | 8 ++++---- cmd/internal-tools/analyze/ranksum.go | 5 +++++ 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/cmd/internal-tools/analyze/analysis_test.go b/cmd/internal-tools/analyze/analysis_test.go index f092054..4f25aee 100644 --- a/cmd/internal-tools/analyze/analysis_test.go +++ b/cmd/internal-tools/analyze/analysis_test.go @@ -320,7 +320,7 @@ func writeCleanCampaign(t *testing.T, directory, name string, budget, steps, run // Arms censored at different budgets are not on the same clock: every clean run // of the wider arm outranks every clean run of the narrower one whatever the -// app did, so the rank-sum and the paired test reach a foregone conclusion the +// app did, so the pairwise and the paired test reach a foregone conclusion the // log-rank in the same report contradicts. groupArms already refuses this // within one arm, and comparing across arms is the same hazard. func TestRun_RefusesToCompareArmsCensoredAtDifferentBudgets(t *testing.T) { diff --git a/cmd/internal-tools/analyze/main.go b/cmd/internal-tools/analyze/main.go index be84c6e..5e1338a 100644 --- a/cmd/internal-tools/analyze/main.go +++ b/cmd/internal-tools/analyze/main.go @@ -56,7 +56,7 @@ func run(arguments []string, stdout, stderr io.Writer) error { flagSet.Var(&directories, "campaign", "campaign directory to read; repeat for more, or pass them as arguments") flagSet.StringVar(&jsonPath, "json", "", "write the machine-readable summary here, or - for stdout") flagSet.StringVar(&question, "question", "", "the research question these campaigns answer; Holm corrects within one invocation, and this records which family that was") - flagSet.BoolVar(&paired, "paired", false, "the two arms ran the same seeds: contrast them seed by seed with the Wilcoxon signed-rank test instead of the rank-sum test") + flagSet.BoolVar(&paired, "paired", false, "the two arms ran the same seeds: contrast them seed by seed with the sign test instead of pooling them into two independent samples") if err := flagSet.Parse(arguments); err != nil { return err } diff --git a/cmd/internal-tools/analyze/planted_test.go b/cmd/internal-tools/analyze/planted_test.go index 58c8484..3882fe8 100644 --- a/cmd/internal-tools/analyze/planted_test.go +++ b/cmd/internal-tools/analyze/planted_test.go @@ -311,12 +311,12 @@ func survivalAtStep(curve []survivalPoint, step float64) (float64, bool) { func TestPlanted_TrueNullIsNotCalledSignificantAboveItsLevel(t *testing.T) { const replicates = 300 model := plantedModel{Hazard: 0.01, Budget: 400} - rejectedByRankSum, rejectedByLogRank := 0, 0 + rejectedByGehan, rejectedByLogRank := 0, 0 for replicate := 0; replicate < replicates; replicate++ { first, second := plantTwoArms(t, int64(1000+replicate), 30, model, model) result := analyseCampaigns(t, first, second) if result.Pairwise[0].PValue < 0.05 { - rejectedByRankSum++ + rejectedByGehan++ } if result.LogRank.PValue < 0.05 { rejectedByLogRank++ @@ -325,7 +325,7 @@ func TestPlanted_TrueNullIsNotCalledSignificantAboveItsLevel(t *testing.T) { // Three standard errors around 0.05 at 300 replicates is 0.05 +/- 0.038. // The lower bound is asserted too: a test that never rejects has bought its // level by losing the power the experiment is sized for. - for name, rejected := range map[string]int{"rank-sum": rejectedByRankSum, "log-rank": rejectedByLogRank} { + for name, rejected := range map[string]int{"gehan": rejectedByGehan, "log-rank": rejectedByLogRank} { rate := float64(rejected) / replicates if rate > 0.09 || rate < 0.015 { t.Errorf("%s called a true null significant in %.1f%% of %d replicates, want about 5%%", @@ -350,7 +350,7 @@ func TestPlanted_TrueNullUnderHeavyCensoringKeepsItsLevel(t *testing.T) { } } if rate := float64(rejected) / replicates; rate > 0.09 { - t.Errorf("rank-sum called a true null significant in %.1f%% of %d replicates under heavy censoring, want at most about 5%%", + t.Errorf("gehan called a true null significant in %.1f%% of %d replicates under heavy censoring, want at most about 5%%", 100*rate, replicates) } } diff --git a/cmd/internal-tools/analyze/ranksum.go b/cmd/internal-tools/analyze/ranksum.go index 118df15..04972e8 100644 --- a/cmd/internal-tools/analyze/ranksum.go +++ b/cmd/internal-tools/analyze/ranksum.go @@ -49,6 +49,11 @@ func vargaDelaneyA12(first, second []float64) float64 { // W. The exact null distribution is used when there are no ties and both // samples are small; otherwise the normal approximation is used with the // continuity correction and the tie correction to the variance. +// +// It takes plain numbers, so it cannot be given campaign runs, where a clean one +// carries a bound and not a value. What it is here for is the uncensored case +// the pipeline's comparison has to reproduce: this implementation is checked +// against R on a published sample, and gehanTest is checked against this one. func rankSum(first, second []float64) rankSumResult { firstSize, secondSize := len(first), len(second) result := rankSumResult{