diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 6bedf380..c463033b 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -51,9 +51,9 @@ jobs: run: >- uv run --frozen --no-build pytest --no-header -q test_bench_stats.py test_bench_measure.py test_bench_runner.py - test_bench_arms.py test_bench_config.py test_sdk_commands.py - test_tdfs_units.py test_encryption_units.py test_sizes_units.py - test_zip64_units.py + test_bench_arms.py test_bench_config.py test_bench_report.py + test_sdk_commands.py test_tdfs_units.py test_encryption_units.py + test_sizes_units.py test_zip64_units.py working-directory: xtest - name: Lint and test otdf-local run: | diff --git a/xtest/perf/report.py b/xtest/perf/report.py index f5e4e8d6..71179440 100644 --- a/xtest/perf/report.py +++ b/xtest/perf/report.py @@ -226,7 +226,8 @@ def markdown( "", f"seed {config.seed}; warm-up {config.warmup} rounds; " f"{config.min_rounds}-{config.max_rounds} measured rounds per cell; " - "stopping on attained CI width, never on significance.", + "stopping on attained CI width, never on significance; " + "p is one-sided, in the direction of the observed ratio.", ] return "\n".join(lines) + "\n" @@ -240,7 +241,14 @@ def _ratio_cell(c: stats.PairedComparison) -> str: def _p_cell(c: stats.PairedComparison) -> str: - p = c.p_adjusted if c.p_adjusted is not None else c.p_value + # Show the one-sided tail matching the observed effect. Below one that is + # the explicit faster-tail test: a BH-adjusted slower-tail p-value cannot + # be read backwards as evidence of improvement, and printing it beside an + # IMPROVED verdict shows ~1.000 next to a significant result. + if c.ratio < 1: + p = c.p_adjusted_faster if c.p_adjusted_faster is not None else c.p_value_faster + else: + p = c.p_adjusted if c.p_adjusted is not None else c.p_value if p is None or not math.isfinite(p): return "-" return f"{p:.3f}" if p >= 0.001 else "<0.001" diff --git a/xtest/test_bench_report.py b/xtest/test_bench_report.py new file mode 100644 index 00000000..bba99f7e --- /dev/null +++ b/xtest/test_bench_report.py @@ -0,0 +1,84 @@ +"""Unit tests for the benchmark report's rendering helpers. + +Offline: no SDKs, no services. Builds `PairedComparison` values directly. +""" + +import math + +import pytest + +from perf import stats +from perf.report import _p_cell +from perf.stats import Verdict + + +def _comparison( + ratio: float, + *, + p_value: float, + p_value_faster: float, + p_adjusted: float | None = None, + p_adjusted_faster: float | None = None, + verdict: Verdict = Verdict.INCONCLUSIVE, +) -> stats.PairedComparison: + return stats.PairedComparison( + n_rounds=20, + baseline_median=1.0, + candidate_median=ratio, + ratio=ratio, + ci_low=ratio * 0.9, + ci_high=ratio * 1.1, + p_value=p_value, + p_value_faster=p_value_faster, + p_adjusted=p_adjusted, + p_adjusted_faster=p_adjusted_faster, + verdict=verdict, + ) + + +class TestPCell: + def test_an_improvement_shows_the_faster_tail(self): + # The slower tail of a 2x speedup is ~1; printing it beside IMPROVED + # contradicts the verdict on the same row. + c = _comparison( + 0.5, + p_value=1.0, + p_value_faster=0.0004, + p_adjusted=1.0, + p_adjusted_faster=0.0011, + verdict=Verdict.IMPROVED, + ) + assert _p_cell(c) == "0.001" + + def test_a_slowdown_shows_the_slower_tail(self): + c = _comparison( + 1.5, + p_value=0.0004, + p_value_faster=1.0, + p_adjusted=0.002, + p_adjusted_faster=1.0, + verdict=Verdict.REGRESSION, + ) + assert _p_cell(c) == "0.002" + + def test_no_change_shows_the_slower_tail(self): + c = _comparison(1.0, p_value=0.5, p_value_faster=0.5, p_adjusted=0.6) + assert _p_cell(c) == "0.600" + + @pytest.mark.parametrize( + ("ratio", "expected"), + [(0.8, "0.030"), (1.2, "0.020")], + ids=["faster", "slower"], + ) + def test_unadjusted_tail_is_used_when_not_corrected(self, ratio, expected): + # Controls are reported unadjusted: they stay out of the BH families. + c = _comparison(ratio, p_value=0.02, p_value_faster=0.03) + assert _p_cell(c) == expected + + def test_tiny_p_is_floored(self): + c = _comparison(0.5, p_value=1.0, p_value_faster=1e-6, p_adjusted_faster=1e-5) + assert _p_cell(c) == "<0.001" + + def test_an_empty_comparison_shows_a_dash(self): + c = _comparison(math.nan, p_value=math.nan, p_value_faster=math.nan) + assert _p_cell(c) == "-"