diff --git a/cmd/internal-tools/analyze/analysis.go b/cmd/internal-tools/analyze/analysis.go index a244c1d..c836a5b 100644 --- a/cmd/internal-tools/analyze/analysis.go +++ b/cmd/internal-tools/analyze/analysis.go @@ -45,11 +45,15 @@ type pairwiseResult struct { Second string `json:"second"` FirstSize int `json:"first_size"` SecondSize int `json:"second_size"` - Statistic float64 `json:"mann_whitney_u"` + Statistic float64 `json:"u"` A12 float64 `json:"a12"` + // Unordered is how many of the run pairs behind U and A12 have no order + // between them, because they were tied or because censoring stopped one run + // before the other violated. Each of them counts as half, so it is also how + // much of the effect size is the null value rather than an observation. + Unordered int `json:"unordered_pairs"` PValue float64 `json:"p_value"` HolmPValue float64 `json:"holm_p_value"` - Exact bool `json:"exact"` } type analysis struct { @@ -197,7 +201,7 @@ func comparePairs(arms []arm) []pairwiseResult { var pairs []pairwiseResult for first := 0; first < len(arms); first++ { for second := first + 1; second < len(arms); second++ { - test := rankSum(arms[first].stepTimes(), arms[second].stepTimes()) + test := gehanTest(arms[first].observations(), arms[second].observations()) pairs = append(pairs, pairwiseResult{ First: arms[first].Name, Second: arms[second].Name, @@ -205,9 +209,9 @@ func comparePairs(arms []arm) []pairwiseResult { SecondSize: test.SecondSize, Statistic: test.Statistic, A12: test.A12, + Unordered: test.Unordered, PValue: test.PValue, HolmPValue: math.NaN(), - Exact: test.Exact, }) } } diff --git a/cmd/internal-tools/analyze/censoring_test.go b/cmd/internal-tools/analyze/censoring_test.go new file mode 100644 index 0000000..5d7b49b --- /dev/null +++ b/cmd/internal-tools/analyze/censoring_test.go @@ -0,0 +1,98 @@ +package main + +import ( + "math" + "path/filepath" + "testing" +) + +// A run stops at whichever comes first, the step budget or the campaign's wall +// clock, so an arm that spends more wall clock per step leaves runs censored far +// below the budget. Those runs are not observations of a violation at that step, +// and the comparison between arms has to read them as the bounds they are. + +type wallClockRun struct { + steps int + violated bool +} + +func stoppedShort(count, steps int) []wallClockRun { + runs := make([]wallClockRun, 0, count) + for index := 0; index < count; index++ { + runs = append(runs, wallClockRun{steps: steps}) + } + return runs +} + +func violatedAt(count, steps int) []wallClockRun { + runs := make([]wallClockRun, 0, count) + for index := 0; index < count; index++ { + runs = append(runs, wallClockRun{steps: steps, violated: true}) + } + return runs +} + +func writeWallClockCampaign(t *testing.T, directory, name string, budget int, runs []wallClockRun) string { + t.Helper() + seeds := make([]int, 0, len(runs)) + records := make([]map[string]any, 0, len(runs)) + for index, run := range runs { + seed := index + 1 + seeds = append(seeds, seed) + record := map[string]any{ + "seed": seed, "exit_code": 0, "steps": run.steps, "actions": run.steps, + "monotonic_millis": 60_000, + } + if run.violated { + record["first_violation_origin_step"] = run.steps + record["violated_properties"] = []string{"plantedProperty"} + } + records = append(records, record) + } + writeCampaign(t, directory, map[string]any{ + "arm": name, "generator": "seeded", "platform": "android", + "max_steps": budget, "seeds": seeds, + }, records) + return directory +} + +func wallClockArms(t *testing.T, early, late []wallClockRun) (string, string) { + t.Helper() + root := t.TempDir() + return writeWallClockCampaign(t, filepath.Join(root, "early"), "early", 400, early), + writeWallClockCampaign(t, filepath.Join(root, "late"), "late", 400, late) +} + +// Twenty runs stopped clean at step 12 against twenty violations at step 100. +// Nothing in the first arm was observed past step 12, so no pair of runs across +// the arms has a determined order and there is no difference to report. +func TestRun_RunsStoppedBeforeEveryEventCarryNoComparison(t *testing.T) { + earlyDirectory, lateDirectory := wallClockArms(t, stoppedShort(20, 12), violatedAt(20, 100)) + pair := analyseCampaigns(t, earlyDirectory, lateDirectory).Pairwise[0] + + if pair.First != "early" || pair.Second != "late" { + t.Fatalf("comparison %s vs %s, want early vs late", pair.First, pair.Second) + } + if math.Abs(pair.A12-0.5) > 1e-12 { + t.Errorf("a12 %.4f between an arm censored at 12 and one violating at 100, want 0.5: "+ + "a run that stopped at step 12 never reached step 100", pair.A12) + } + if pair.PValue < 0.05 { + t.Errorf("p %.3e, want no significant difference: the arms were never observed over the same steps", + pair.PValue) + } +} + +// Where the two arms were observed together, over the first twelve steps, the +// arm the wall clock stopped is the one that did not violate. The effect size +// has to follow that and not the step counts the flattening reads. +func TestRun_EffectSizeFollowsWhatCensoringDetermines(t *testing.T) { + late := append(violatedAt(6, 5), violatedAt(14, 100)...) + earlyDirectory, lateDirectory := wallClockArms(t, stoppedShort(20, 12), late) + pair := analyseCampaigns(t, earlyDirectory, lateDirectory).Pairwise[0] + + if pair.A12 <= 0.5 { + t.Errorf("a12 %.4f, want above 0.5: six of the late arm's runs violated by step 5 "+ + "and none of the early arm's twenty had violated by step 12", pair.A12) + } +} diff --git a/cmd/internal-tools/analyze/end_to_end_test.go b/cmd/internal-tools/analyze/end_to_end_test.go index 9278e42..c2a7ebb 100644 --- a/cmd/internal-tools/analyze/end_to_end_test.go +++ b/cmd/internal-tools/analyze/end_to_end_test.go @@ -92,7 +92,7 @@ func TestRun_EndToEndOverFixtureCampaignDirectories(t *testing.T) { for _, fragment := range []string{ "steps to first violation, right-censored at the last step a clean run reached", "log-rank across 2 arms", - "pairwise wilcoxon rank-sum", + "pairwise gehan generalized wilcoxon", "llm vs seeded", "excluded 1 run(s) as missing data", } { @@ -185,8 +185,10 @@ func TestRun_EndToEndOverFixtureCampaignDirectories(t *testing.T) { if pair.HolmPValue != pair.PValue { t.Errorf("holm p %v differs from raw p %v in a family of one", pair.HolmPValue, pair.PValue) } - if pair.Exact { - t.Error("used the exact null distribution despite the tie mass at the budget") + // Six seeded runs and one llm run ran the budget clean, and two censored + // runs have no order between them whatever step either stopped on. + if pair.Unordered < 6 { + t.Errorf("%d unordered pair(s), want at least the six pairs of censored runs", pair.Unordered) } } diff --git a/cmd/internal-tools/analyze/load.go b/cmd/internal-tools/analyze/load.go index 21ef31d..2bb8796 100644 --- a/cmd/internal-tools/analyze/load.go +++ b/cmd/internal-tools/analyze/load.go @@ -328,15 +328,3 @@ func observationOf(item classifiedRun, budget int) observation { // the steps it never ran are not exposure it survived. return observation{Steps: float64(min(item.Steps, budget)), Event: false} } - -// stepTimes is the observations flattened to plain numbers, censored runs held -// at the steps they ran. Holding them there rather than dropping them is -// conservative: it can only understate how much sooner a violating arm finds -// its first defect, never overstate it. -func (a arm) stepTimes() []float64 { - var result []float64 - for _, item := range a.observations() { - result = append(result, item.Steps) - } - return result -} diff --git a/cmd/internal-tools/analyze/planted_test.go b/cmd/internal-tools/analyze/planted_test.go index 81ede43..2dde2fd 100644 --- a/cmd/internal-tools/analyze/planted_test.go +++ b/cmd/internal-tools/analyze/planted_test.go @@ -401,11 +401,11 @@ func TestPlanted_HeavyCensoringLeavesTheMedianUndefinedAndKeepsTheEffect(t *test } } -// Every censored run is recorded at the same step, so an arm at a real budget -// is mostly one enormous tied group. The exact null distribution is not -// available there and the tie-corrected normal approximation has to be, which -// is checked against a permutation p-value over the pipeline's own two samples. -func TestPlanted_TiesAtTheBudgetUseTheTieCorrectedApproximation(t *testing.T) { +// An arm at a real budget is mostly one enormous group of runs censored +// together at it. No exact null distribution covers that, so the conditional +// variance carries the p-value, and it is checked against a permutation p-value +// over the pipeline's own two samples. +func TestPlanted_CensoringAtTheBudgetTracksThePermutationPValue(t *testing.T) { cases := []struct { sourceSeed int64 quiet, loud float64 @@ -429,45 +429,43 @@ func TestPlanted_TiesAtTheBudgetUseTheTieCorrectedApproximation(t *testing.T) { loudDirectory := writePlantedCampaign(t, filepath.Join(root, "loud"), "loud", loud.Budget, loudRuns) pair := analyseCampaigns(t, loudDirectory, quietDirectory).Pairwise[0] - if pair.Exact { - t.Error("used the exact null distribution on samples tied at the budget") - } - permuted := permutationRankSumTwoSided(recordedSteps(loudRuns, loud.Budget), recordedSteps(quietRuns, quiet.Budget)) + permuted := permutationGehanTwoSided(plantedObservations(loudRuns), plantedObservations(quietRuns)) if permuted > 0.01 { discriminating++ } - if math.Abs(pair.PValue-permuted) > 0.01 { + // The conditional variance and the permutation variance are two + // estimators and not one, so they are near rather than equal. Over these + // four samples the gap is at most 0.011, at a p-value of 0.49 where + // nothing is decided; where a decision is made it is under 0.005. A + // variance wrong by a factor moves the p-value across orders of + // magnitude, which is what this catches. + if math.Abs(pair.PValue-permuted) > 0.015 { t.Errorf("hazards %v against %v: p-value %.4f, want near the permutation p-value %.4f", test.quiet, test.loud, pair.PValue, permuted) } } - // Dropping the tie term moves a p-value by roughly a hundredth here, which - // only shows against a permutation p-value that is not already at the - // floor, so the check has to include cases that are not overwhelming. + // A permutation p-value already at the floor cannot show a variance moving + // by a fraction, so the check has to include cases that are not overwhelming. if discriminating < 3 { t.Fatalf("%d of %d cases carry a p-value large enough to discriminate", discriminating, len(cases)) } } -func recordedSteps(runs []plantedRun, budget int) []float64 { - values := make([]float64, 0, len(runs)) +func plantedObservations(runs []plantedRun) []observation { + items := make([]observation, 0, len(runs)) for _, run := range runs { - if run.violated { - values = append(values, float64(run.steps)) - continue - } - values = append(values, float64(budget)) + items = append(items, observation{Steps: float64(run.steps), Event: run.violated}) } - return values + return items } -// permutationRankSumTwoSided is the randomization p-value of the Mann-Whitney -// statistic: relabel the pooled sample many times and count how often the -// statistic lands at least as far from its null mean as the observed one. -func permutationRankSumTwoSided(first, second []float64) float64 { - pooled := append(append([]float64{}, first...), second...) - observed := rankSum(first, second).Statistic - mean := float64(len(first)) * float64(len(second)) / 2 +// permutationGehanTwoSided is the randomization p-value of Gehan's statistic: +// relabel the pooled runs many times, censoring and all, and count how often the +// statistic lands at least as far from zero as the observed one. Both arms here +// censor on the same schedule, which is what makes relabelling them a null. +func permutationGehanTwoSided(first, second []observation) float64 { + pooled := append(append([]observation{}, first...), second...) + observed, _ := gehanReference(first, second) source := rand.New(rand.NewSource(99)) const shuffles = 20000 extreme := 0 @@ -475,8 +473,8 @@ func permutationRankSumTwoSided(first, second []float64) float64 { source.Shuffle(len(pooled), func(left, right int) { pooled[left], pooled[right] = pooled[right], pooled[left] }) - statistic := rankSum(pooled[:len(first)], pooled[len(first):]).Statistic - if math.Abs(statistic-mean) >= math.Abs(observed-mean)-1e-9 { + statistic, _ := gehanReference(pooled[:len(first)], pooled[len(first):]) + if math.Abs(statistic) >= math.Abs(observed)-1e-9 { extreme++ } } diff --git a/cmd/internal-tools/analyze/report.go b/cmd/internal-tools/analyze/report.go index c47d5aa..3eecd95 100644 --- a/cmd/internal-tools/analyze/report.go +++ b/cmd/internal-tools/analyze/report.go @@ -107,9 +107,10 @@ func writeReport(result analysis, out io.Writer) { } if len(result.Pairwise) > 0 { - fmt.Fprintln(out, "\npairwise wilcoxon rank-sum, censored runs held at the steps they ran") - fmt.Fprintln(out, "a12 above 0.5 means the first arm takes more steps to its first violation") - writeTable(out, []string{"comparison", "n1", "n2", "u", "a12", "p", "holm p"}, func(add func(...string)) { + fmt.Fprintln(out, "\npairwise gehan generalized wilcoxon, which reads a censored run as the bound it is") + fmt.Fprintln(out, "a12 above 0.5 means the first arm takes more steps to its first violation; u counts the run") + fmt.Fprintln(out, "pairs the first arm outlived, and the unordered pairs count as half in both u and a12") + writeTable(out, []string{"comparison", "n1", "n2", "u", "a12", "unordered", "p", "holm p"}, func(add func(...string)) { for _, pair := range result.Pairwise { add( pair.First+" vs "+pair.Second, @@ -117,6 +118,7 @@ func writeReport(result analysis, out io.Writer) { strconv.Itoa(pair.SecondSize), fmt.Sprintf("%.1f", pair.Statistic), fmt.Sprintf("%.3f", pair.A12), + fmt.Sprintf("%d of %d", pair.Unordered, pair.FirstSize*pair.SecondSize), formatPValue(pair.PValue), formatPValue(pair.HolmPValue), )