From 0cec68bc30c7824de8196b7e397c9ec54c2f9099 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Tue, 15 Sep 2026 14:51:33 +0000 Subject: [PATCH] fix: show uncertainty in benchmark comparisons Signed-off-by: Gregor Zeitlinger --- .mise/tasks/generate_benchmark_summary.py | 56 +++++++---- .../tasks/test_generate-benchmark-summary.py | 94 ++++++++++++++++++- 2 files changed, 127 insertions(+), 23 deletions(-) diff --git a/.mise/tasks/generate_benchmark_summary.py b/.mise/tasks/generate_benchmark_summary.py index 0da622bcbf..ce591a3b56 100755 --- a/.mise/tasks/generate_benchmark_summary.py +++ b/.mise/tasks/generate_benchmark_summary.py @@ -204,7 +204,7 @@ def get_commit_sha(provided_sha: str | None) -> str: def format_score(score) -> str: """Format score with appropriate precision.""" if score is None: - return "" + return "—" try: val = float(score) if val >= 1_000_000: @@ -340,14 +340,18 @@ def comparison_status(head: dict, baseline: dict) -> str: return "inconclusive" change = performance_change(head, baseline) - if change is None or head_interval is None or baseline_interval is None: + if change is None: return "inconclusive" + if head_interval is None or baseline_interval is None: + return "inconclusive (missing or invalid confidence interval)" head_low, head_high = head_interval baseline_low, baseline_high = baseline_interval intervals_overlap = head_low <= baseline_high and baseline_low <= head_high - if intervals_overlap or abs(change) < PRACTICAL_CHANGE_THRESHOLD: - return "within noise" + if intervals_overlap: + return "inconclusive (overlapping intervals)" + if abs(change) < PRACTICAL_CHANGE_THRESHOLD: + return f"below {PRACTICAL_CHANGE_THRESHOLD:g}% threshold" return "meaningful improvement" if change > 0 else "meaningful regression" @@ -375,6 +379,16 @@ def format_change(change: float | None) -> str: return f"{change:+.1f}%" +def format_score_with_interval(result: dict) -> str: + """Format a score together with its JMH confidence interval.""" + score = metric_score(result) + interval = score_interval(result) + if score is None or interval is None: + return format_score(score) + low, high = interval + return f"{format_score(score)} [{format_score(low)}, {format_score(high)}]" + + def metric_direction_note(results: list) -> str: """Describe whether scores represent throughput or latency.""" directions = { @@ -420,6 +434,11 @@ def generate_comparison_section( md.append(f"- **Head:** {format_commit_link(commit_sha, repo)}") md.append(f"- **Base:** {format_commit_link(baseline_sha, baseline_repo)}") md.append(f"- **Metric direction:** {metric_direction_note(results)}") + md.append( + "- **Uncertainty:** values include JMH 99.9% confidence intervals; verdicts use " + "interval overlap and a practical-change threshold as a conservative heuristic, " + "not as a statistical significance test." + ) if comparison_note: md.append(f"- **Note:** {comparison_note}") if baseline_system_info: @@ -436,19 +455,19 @@ def generate_comparison_section( md.append("") return md - md.append("| Benchmark | PR | Base | Head vs base | Regression verdict |") - md.append("|:----------|---:|-----:|-------:|:-------|") + md.append( + "| Benchmark | PR (99.9% CI) | Base (99.9% CI) | Head vs base | Regression verdict |" + ) + md.append("|:----------|--------------:|----------------:|-------:|:-------|") for name in common_names: head = by_name[name] baseline = baseline_by_name[name] - head_score = metric_score(head) - baseline_score = metric_score(baseline) md.append( "| " f"{short_benchmark_name(name)} | " - f"{format_score(head_score)} | " - f"{format_score(baseline_score)} | " + f"{format_score_with_interval(head)} | " + f"{format_score_with_interval(baseline)} | " f"{format_change(performance_change(head, baseline))} | " f"{comparison_status(head, baseline)} |" ) @@ -588,8 +607,8 @@ def generate_markdown( for b in sorted_benchmarks: name = b.get("benchmark", "").split(".")[-1] - score = b.get("primaryMetric", {}).get("score", 0) - error = b.get("primaryMetric", {}).get("scoreError", 0) + score = b.get("primaryMetric", {}).get("score") + error = b.get("primaryMetric", {}).get("scoreError") unit = b.get("primaryMetric", {}).get("scoreUnit", "ops/s") score_fmt = format_score(score) @@ -611,8 +630,8 @@ def generate_markdown( md.append("|:----------|------:|------:|:------|") for b in sorted(head_only_results, key=lambda x: x.get("benchmark", "")): name = short_benchmark_name(b.get("benchmark", "")) - score = b.get("primaryMetric", {}).get("score", 0) - error = b.get("primaryMetric", {}).get("scoreError", 0) + score = b.get("primaryMetric", {}).get("score") + error = b.get("primaryMetric", {}).get("scoreError") unit = b.get("primaryMetric", {}).get("scoreUnit", "ops/s") md.append( f"| {name} | {format_score(score)} | {format_error(error)} | {unit} |" @@ -630,14 +649,14 @@ def generate_markdown( name = short_benchmark_name(b.get("benchmark", "")) mode = b.get("mode", "thrpt") cnt = b.get("measurementIterations", 0) * b.get("forks", 1) - score = b.get("primaryMetric", {}).get("score", 0) - error = b.get("primaryMetric", {}).get("scoreError", 0) + score = b.get("primaryMetric", {}).get("score") + error = b.get("primaryMetric", {}).get("scoreError") unit = b.get("primaryMetric", {}).get("scoreUnit", "ops/s") try: score_str = f"{float(score):.3f}" except (ValueError, TypeError): - score_str = str(score) + score_str = format_score(score) try: error_val = float(error) @@ -667,7 +686,8 @@ def generate_markdown( "- **Regression verdict** requires comparable benchmark metadata, " "non-overlapping JMH confidence intervals, and a change of at least " f"{PRACTICAL_CHANGE_THRESHOLD:.0f}%; otherwise it is marked " - '"within noise" or "inconclusive".' + '"below the practical threshold" or "inconclusive". This is a ' + "conservative heuristic, not a statistical significance test." ) md.append( "- Scores for different benchmark methods are not ranked against one another; " diff --git a/.mise/tasks/test_generate-benchmark-summary.py b/.mise/tasks/test_generate-benchmark-summary.py index 9918d0e49c..2f3d26454a 100644 --- a/.mise/tasks/test_generate-benchmark-summary.py +++ b/.mise/tasks/test_generate-benchmark-summary.py @@ -56,10 +56,31 @@ def test_meaningful_regression_requires_threshold_and_separation(self): comparison_status(result(score=94), result()), "meaningful regression" ) - def test_small_or_uncertain_change_is_within_noise(self): + def test_overlapping_intervals_are_inconclusive(self): self.assertEqual( comparison_status(result(score=102, error=5), result(error=5)), - "within noise", + "inconclusive (overlapping intervals)", + ) + + def test_overlapping_intervals_are_inconclusive_even_for_large_change(self): + head = result(score=9946.81, error=None) + base = result(score=7239.716, error=None) + head["primaryMetric"]["scoreConfidence"] = [8459.55, 11434.08] + base["primaryMetric"]["scoreConfidence"] = [5298.4, 9181.03] + self.assertEqual( + comparison_status(head, base), "inconclusive (overlapping intervals)" + ) + + def test_non_overlapping_change_below_threshold_is_below_threshold(self): + self.assertEqual( + comparison_status(result(score=103, error=0.1), result(error=0.1)), + "below 5% threshold", + ) + + def test_non_overlapping_change_is_meaningful(self): + self.assertEqual( + comparison_status(result(score=110, error=0.1), result(error=0.1)), + "meaningful improvement", ) def test_mismatched_metadata_is_inconclusive(self): @@ -104,11 +125,27 @@ def test_invalid_fallback_uncertainty_is_inconclusive(self): for error in (float("inf"), float("-inf"), -1.0): head = result(error=error) head["primaryMetric"].pop("scoreConfidence") - self.assertEqual(comparison_status(head, result()), "inconclusive", error) + self.assertEqual( + comparison_status(head, result()), + "inconclusive (missing or invalid confidence interval)", + error, + ) def test_missing_confidence_interval_is_inconclusive(self): head = result(score=106, error=None) - self.assertEqual(comparison_status(head, result()), "inconclusive") + self.assertEqual( + comparison_status(head, result()), + "inconclusive (missing or invalid confidence interval)", + ) + + def test_invalid_confidence_interval_is_inconclusive(self): + head = result(score=106, error=1) + head["primaryMetric"]["scoreConfidence"] = [0, float("nan")] + head["primaryMetric"].pop("scoreError") + self.assertEqual( + comparison_status(head, result()), + "inconclusive (missing or invalid confidence interval)", + ) class TestBenchmarkMarkdown(unittest.TestCase): @@ -128,7 +165,8 @@ def test_head_only_benchmarks_are_separate_and_not_ranked(self): ) self.assertIn( - "| Benchmark | PR | Base | Head vs base | Regression verdict |", markdown + "| Benchmark | PR (99.9% CI) | Base (99.9% CI) | Head vs base | Regression verdict |", + markdown, ) self.assertIn("## New benchmarks in PR head", markdown) self.assertIn("no base counterpart", markdown) @@ -136,6 +174,52 @@ def test_head_only_benchmarks_are_separate_and_not_ranked(self): self.assertNotIn("Within run", markdown) self.assertNotIn("x slower", markdown) + def test_comparison_table_shows_confidence_intervals_and_uncertainty_note(self): + markdown = generate_markdown( + [result(score=110, error=1)], + "head", + "prometheus/client_java", + baseline_results=[result(score=100, error=1)], + baseline_sha="base", + baseline_repo="prometheus/client_java", + ) + self.assertIn("110.00 [109.00, 111.00]", markdown) + self.assertIn("100.00 [99.00, 101.00]", markdown) + self.assertIn("not as a statistical significance test", markdown) + + def test_real_overlapping_example_is_rendered_with_intervals(self): + head = result(score=9946.81, error=None) + base = result(score=7239.716, error=None) + head["primaryMetric"]["scoreConfidence"] = [8459.55, 11434.08] + base["primaryMetric"]["scoreConfidence"] = [5298.4, 9181.03] + markdown = generate_markdown( + [head], + "head", + "prometheus/client_java", + baseline_results=[base], + baseline_sha="base", + baseline_repo="prometheus/client_java", + ) + self.assertIn("9.95K [8.46K, 11.43K]", markdown) + self.assertIn("7.24K [5.30K, 9.18K]", markdown) + self.assertIn("inconclusive (overlapping intervals)", markdown) + + def test_missing_values_are_not_rendered_as_zero(self): + missing_interval = result(score=106, error=None) + missing_score = result(score=100) + missing_score["primaryMetric"].pop("score") + markdown = generate_markdown( + [missing_interval, missing_score], + "head", + "prometheus/client_java", + baseline_results=[result(), result()], + baseline_sha="base", + baseline_repo="prometheus/client_java", + ) + self.assertIn("106.00 |", markdown) + self.assertIn("— |", markdown) + self.assertNotIn("| 0.00 |", markdown) + def test_latency_note_is_mode_aware(self): base = result() head = result()