mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-02 11:07:10 +00:00
fix(analyze): write an undefined paired p-value as null, not as NaN
A paired contrast where censoring orders no pair has no p-value, and JSON has no NaN, so --json failed with 'marshal summary: json: unsupported value: NaN' and wrote no summary at all after printing a complete report. The two fields join the medians and the rates already carried as pointers, undefined reading as null in the summary and n/a in the report. Reachable since a clean run started being censored where it stopped: an arm the wall clock stops before its partner ever violates orders nothing.
This commit is contained in:
1 parent
8fad1937bb
commit
4bcd82a8a6
6 files changed
+60
-23
No files matched your search
@@ -105,8 +105,9 @@ func analysePaired(arms []arm, now time.Time) (analysis, error) {
|
|||||||
return analysis{}, fmt.Errorf("arms %q and %q share no seed with a usable run in both",
|
return analysis{}, fmt.Errorf("arms %q and %q share no seed with a usable run in both",
|
||||||
testable[0].Name, testable[1].Name)
|
testable[0].Name, testable[1].Name)
|
||||||
}
|
}
|
||||||
if !math.IsNaN(comparison.PValue) {
|
if comparison.PValue != nil {
|
||||||
comparison.HolmPValue = holm([]float64{comparison.PValue})[0]
|
adjusted := holm([]float64{*comparison.PValue})[0]
|
||||||
|
comparison.HolmPValue = &adjusted
|
||||||
result.HolmFamilySize = 1
|
result.HolmFamilySize = 1
|
||||||
}
|
}
|
||||||
result.Paired = &comparison
|
result.Paired = &comparison
|
||||||
|
|||||||
@@ -1,8 +1,11 @@
|
|||||||
package main
|
package main
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
|
"io"
|
||||||
"math"
|
"math"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -116,3 +119,30 @@ func TestRun_PairedContrastFollowsWhatCensoringDetermines(t *testing.T) {
|
|||||||
t.Errorf("a12 within pairs %.4f, want above 0.5", paired.A12)
|
t.Errorf("a12 within pairs %.4f, want above 0.5", paired.A12)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Nothing orders any pair here, so there is no test to report. The summary has
|
||||||
|
// to say that rather than failing to write a number that does not exist: a
|
||||||
|
// NaN p-value is not JSON and the whole summary went unwritten behind it.
|
||||||
|
func TestRun_PairedContrastWithNoOrderedPairSaysSo(t *testing.T) {
|
||||||
|
earlyDirectory, lateDirectory := wallClockArms(t, stoppedShort(20, 12), violatedAt(20, 100))
|
||||||
|
result := analyseCampaigns(t, "--paired", earlyDirectory, lateDirectory)
|
||||||
|
|
||||||
|
paired := *result.Paired
|
||||||
|
if paired.Pairs != 20 || paired.Unordered != 20 {
|
||||||
|
t.Fatalf("paired %+v, want twenty pairs and all of them unordered", paired)
|
||||||
|
}
|
||||||
|
if paired.PValue != nil || paired.HolmPValue != nil {
|
||||||
|
t.Errorf("p %v and holm p %v, want both undefined", paired.PValue, paired.HolmPValue)
|
||||||
|
}
|
||||||
|
if result.HolmFamilySize != 0 {
|
||||||
|
t.Errorf("holm family of %d, want none where nothing was tested", result.HolmFamilySize)
|
||||||
|
}
|
||||||
|
|
||||||
|
var stdout bytes.Buffer
|
||||||
|
if err := run([]string{"--paired", earlyDirectory, lateDirectory}, &stdout, io.Discard); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if !strings.Contains(stdout.String(), "sign test over the 0 ordered pair(s), p n/a, holm p n/a") {
|
||||||
|
t.Errorf("report does not say the test was not run:\n%s", stdout.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -65,9 +65,12 @@ type pairedComparison struct {
|
|||||||
// of matched seeds on which the first arm took more steps, an unordered pair
|
// of matched seeds on which the first arm took more steps, an unordered pair
|
||||||
// counting as half. A matched design has no reason to compare the two arms
|
// counting as half. A matched design has no reason to compare the two arms
|
||||||
// as pooled bags of runs when each seed has a partner.
|
// as pooled bags of runs when each seed has a partner.
|
||||||
A12 float64 `json:"a12_within_pairs"`
|
A12 float64 `json:"a12_within_pairs"`
|
||||||
PValue float64 `json:"p_value"`
|
// PValue is undefined, and null in the summary, when censoring left no pair
|
||||||
HolmPValue float64 `json:"holm_p_value"`
|
// ordered: there is nothing for the test to be a test of, and JSON has no
|
||||||
|
// way to write the number that is not there.
|
||||||
|
PValue *float64 `json:"p_value"`
|
||||||
|
HolmPValue *float64 `json:"holm_p_value"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// pairArms matches the two arms by seed and contrasts them pair by pair. A seed
|
// pairArms matches the two arms by seed and contrasts them pair by pair. A seed
|
||||||
@@ -84,13 +87,7 @@ func pairArms(first, second arm) (pairedComparison, error) {
|
|||||||
return pairedComparison{}, err
|
return pairedComparison{}, err
|
||||||
}
|
}
|
||||||
|
|
||||||
comparison := pairedComparison{
|
comparison := pairedComparison{First: first.Name, Second: second.Name, A12: math.NaN()}
|
||||||
First: first.Name,
|
|
||||||
Second: second.Name,
|
|
||||||
A12: math.NaN(),
|
|
||||||
PValue: math.NaN(),
|
|
||||||
HolmPValue: math.NaN(),
|
|
||||||
}
|
|
||||||
var differences []float64
|
var differences []float64
|
||||||
for _, seed := range sortedSeeds(firstBySeed, secondBySeed) {
|
for _, seed := range sortedSeeds(firstBySeed, secondBySeed) {
|
||||||
left, inFirst := firstBySeed[seed]
|
left, inFirst := firstBySeed[seed]
|
||||||
@@ -130,7 +127,9 @@ func pairArms(first, second arm) (pairedComparison, error) {
|
|||||||
comparison.Sign = -1
|
comparison.Sign = -1
|
||||||
}
|
}
|
||||||
comparison.A12 = (float64(comparison.SecondSooner) + 0.5*float64(comparison.Unordered)) / float64(comparison.Pairs)
|
comparison.A12 = (float64(comparison.SecondSooner) + 0.5*float64(comparison.Unordered)) / float64(comparison.Pairs)
|
||||||
comparison.PValue = signTest(comparison.FirstSooner, comparison.SecondSooner)
|
if tested := signTest(comparison.FirstSooner, comparison.SecondSooner); !math.IsNaN(tested) {
|
||||||
|
comparison.PValue = &tested
|
||||||
|
}
|
||||||
return comparison, nil
|
return comparison, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -87,7 +87,7 @@ func TestPairArms_ScoresEachPairByWhichRunOutlivedTheOther(t *testing.T) {
|
|||||||
t.Errorf("median difference %v over %d pair(s), want 10 over 2",
|
t.Errorf("median difference %v over %d pair(s), want 10 over 2",
|
||||||
comparison.MedianDifference, comparison.BothViolated)
|
comparison.MedianDifference, comparison.BothViolated)
|
||||||
}
|
}
|
||||||
if want := signTest(0, 2); comparison.PValue != want {
|
if want := signTest(0, 2); comparison.PValue == nil || *comparison.PValue != want {
|
||||||
t.Errorf("p %v, want the sign test's %v over the two ordered pairs", comparison.PValue, want)
|
t.Errorf("p %v, want the sign test's %v over the two ordered pairs", comparison.PValue, want)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -105,8 +105,8 @@ func TestPairArms_PairsOfCleanRunsAreNotEvidence(t *testing.T) {
|
|||||||
if comparison.Unordered != 2 || comparison.Sign != 0 {
|
if comparison.Unordered != 2 || comparison.Sign != 0 {
|
||||||
t.Errorf("comparison %+v, want both pairs unordered and no direction", comparison)
|
t.Errorf("comparison %+v, want both pairs unordered and no direction", comparison)
|
||||||
}
|
}
|
||||||
if !math.IsNaN(comparison.PValue) {
|
if comparison.PValue != nil {
|
||||||
t.Errorf("p %v, want undefined with no ordered pair", comparison.PValue)
|
t.Errorf("p %v, want undefined with no ordered pair", *comparison.PValue)
|
||||||
}
|
}
|
||||||
if comparison.MedianDifference != nil {
|
if comparison.MedianDifference != nil {
|
||||||
t.Errorf("median difference %v, want undefined where no pair has two violations",
|
t.Errorf("median difference %v, want undefined where no pair has two violations",
|
||||||
@@ -181,8 +181,8 @@ func TestPairArms_DirectionReversesWithTheArms(t *testing.T) {
|
|||||||
t.Errorf("median differences %v and %v, want opposites",
|
t.Errorf("median differences %v and %v, want opposites",
|
||||||
*forward.MedianDifference, *reversed.MedianDifference)
|
*forward.MedianDifference, *reversed.MedianDifference)
|
||||||
}
|
}
|
||||||
if math.Abs(forward.PValue-reversed.PValue) > 1e-12 {
|
if math.Abs(*forward.PValue-*reversed.PValue) > 1e-12 {
|
||||||
t.Errorf("p-values %v and %v, want the same two-sided value", forward.PValue, reversed.PValue)
|
t.Errorf("p-values %v and %v, want the same two-sided value", *forward.PValue, *reversed.PValue)
|
||||||
}
|
}
|
||||||
if math.Abs(forward.A12+reversed.A12-1) > 1e-12 {
|
if math.Abs(forward.A12+reversed.A12-1) > 1e-12 {
|
||||||
t.Errorf("a12 %v and %v, want them to sum to 1", forward.A12, reversed.A12)
|
t.Errorf("a12 %v and %v, want them to sum to 1", forward.A12, reversed.A12)
|
||||||
|
|||||||
@@ -647,11 +647,11 @@ func TestPlanted_PairedComparisonRecoversTheShiftAndItsSign(t *testing.T) {
|
|||||||
if want := 0.5 * float64(unordered) / 30; paired.A12 != want {
|
if want := 0.5 * float64(unordered) / 30; paired.A12 != want {
|
||||||
t.Errorf("a12 within pairs %v, want %v where no pair favours the first arm", paired.A12, want)
|
t.Errorf("a12 within pairs %v, want %v where no pair favours the first arm", paired.A12, want)
|
||||||
}
|
}
|
||||||
if paired.PValue > 0.001 {
|
if paired.PValue == nil || *paired.PValue > 0.001 {
|
||||||
t.Errorf("p-value %v for a shift planted in every pair", paired.PValue)
|
t.Errorf("p-value %v for a shift planted in every pair", paired.PValue)
|
||||||
}
|
}
|
||||||
if paired.HolmPValue != paired.PValue {
|
if *paired.HolmPValue != *paired.PValue {
|
||||||
t.Errorf("holm p %v in a family of one, want the raw %v", paired.HolmPValue, paired.PValue)
|
t.Errorf("holm p %v in a family of one, want the raw %v", *paired.HolmPValue, *paired.PValue)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -665,7 +665,7 @@ func TestPlanted_PairedNullIsNotCalledSignificantAboveItsLevel(t *testing.T) {
|
|||||||
for replicate := 0; replicate < replicates; replicate++ {
|
for replicate := 0; replicate < replicates; replicate++ {
|
||||||
first, second := plantTwoArms(t, int64(5000+replicate), 30, model, model)
|
first, second := plantTwoArms(t, int64(5000+replicate), 30, model, model)
|
||||||
result := analyseCampaigns(t, "--paired", first, second)
|
result := analyseCampaigns(t, "--paired", first, second)
|
||||||
if result.Paired.PValue < 0.05 {
|
if *result.Paired.PValue < 0.05 {
|
||||||
rejected++
|
rejected++
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -149,7 +149,7 @@ func writePaired(out io.Writer, comparison pairedComparison) {
|
|||||||
formatStepDifference(comparison.MedianDifference), comparison.BothViolated, comparison.Sign, comparison.A12)
|
formatStepDifference(comparison.MedianDifference), comparison.BothViolated, comparison.Sign, comparison.A12)
|
||||||
fmt.Fprintf(out, "sign test over the %d ordered pair(s), p %s, holm p %s\n",
|
fmt.Fprintf(out, "sign test over the %d ordered pair(s), p %s, holm p %s\n",
|
||||||
comparison.FirstSooner+comparison.SecondSooner,
|
comparison.FirstSooner+comparison.SecondSooner,
|
||||||
formatPValue(comparison.PValue), formatPValue(comparison.HolmPValue))
|
formatOptionalPValue(comparison.PValue), formatOptionalPValue(comparison.HolmPValue))
|
||||||
if len(comparison.UnpairedSeeds) > 0 {
|
if len(comparison.UnpairedSeeds) > 0 {
|
||||||
fmt.Fprintf(out, "%d seed(s) usable in one arm only and left out of the pairing: %v\n",
|
fmt.Fprintf(out, "%d seed(s) usable in one arm only and left out of the pairing: %v\n",
|
||||||
len(comparison.UnpairedSeeds), comparison.UnpairedSeeds)
|
len(comparison.UnpairedSeeds), comparison.UnpairedSeeds)
|
||||||
@@ -209,6 +209,13 @@ func formatSingletons(summary armSummary) string {
|
|||||||
return fmt.Sprintf("%d/%d (%.3f)", summary.SingletonDefects, summary.DistinctDefects, *summary.SingletonFraction)
|
return fmt.Sprintf("%d/%d (%.3f)", summary.SingletonDefects, summary.DistinctDefects, *summary.SingletonFraction)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func formatOptionalPValue(value *float64) string {
|
||||||
|
if value == nil {
|
||||||
|
return "n/a"
|
||||||
|
}
|
||||||
|
return formatPValue(*value)
|
||||||
|
}
|
||||||
|
|
||||||
func formatPValue(value float64) string {
|
func formatPValue(value float64) string {
|
||||||
switch {
|
switch {
|
||||||
case math.IsNaN(value):
|
case math.IsNaN(value):
|
||||||
|
|||||||
Reference in new issue
Block a user