Skip to content

fix: report standard medians for even benchmark samples - #28

Merged
cppla merged 1 commit into
mainfrom
codex/benchmark-even-median
Sep 23, 2026
Merged

cppla merged 1 commit into
mainfrom
codex/benchmark-even-median

Conversation

@cppla

@cppla cppla commented Sep 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Correct median_mbps and median_duration_ms for even sample counts by averaging the two middle observations.
  • Keep the existing p05/p95 nearest-order-statistic convention unchanged; document both conventions and how to recompute older even-count reports from raw samples.
  • Avoid overflowing the intermediate sum for large finite, nonnegative measurements.

Regression evidence

  • Before the fix, the new test reproduced median([2,10]) = 10 instead of 6, and median([1,2,8,100]) = 8 instead of 5.
  • Added empty/single/odd/even/repeated/large-finite cases and explicit unchanged tail-percentile checks.
  • Targeted race tests x10, full make check, full make race, formatting and independent review passed.
  • Exact head 2f4b67dca7cf744d8f4293f457c8db72a645054f: CI 35811582485 (all 11 jobs including container integration and OCI), CodeQL 35811582479, and Linux netem 35811582480 completed successfully.
  • Independent review and automated PR review found no remaining issues; no unresolved review threads.

Scope

Benchmark reporting and documentation only. No transport/controller defaults, wire protocol, dependency, release or tag change. Raw samples and existing experimental binaries/data were not rewritten. This is a statistics correction, not a throughput improvement claim.

Merge verification

  • Squash-merged as eecd1d992c46124c3390c0c9ddd918f74d47e011 on main; its tree matches the tested PR head (07a234e7b47d57942b73fffc2ac079d6495e36dc).
  • Exact-main CI 35811805160: all 11 jobs passed, including actual container integration, OCI image-index build, race detector and Windows UDP continuity steps.
  • Exact-main CodeQL 35811805240 and Linux netem 35811805277 passed; the namespace/netem execution step completed successfully.
  • Main working tree is clean. No release or tag created.

Copilot AI lite review requested due to automatic review settings September 23, 2026 02:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The reviewed implementation, documentation, and regression coverage have no unresolved blocking issues.

Review effort: Lite
Findings: None

What changed in this PR

Corrects benchmark medians for even-sized samples while preserving existing percentile behavior.

Changes:

  • Adds overflow-safe averaging of the two middle observations.
  • Documents median and percentile conventions.
  • Adds regression coverage for edge cases and unchanged tail percentiles.
File Summary
docs/​BENCHMARK.md Documents statistical conventions and legacy report handling.
cmd/​autocar/​bench.go Implements corrected overflow-safe medians.
cmd/​autocar/​bench_median_test.go Adds median and percentile regression coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@cppla
cppla merged commit eecd1d9 into main Sep 23, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants