Skip to content

feat(xtest): compare benchmark candidates with each other - #659

Draft
dmihalcik-virtru wants to merge 1 commit into
DSPX-4372-s3-multi-armfrom
DSPX-4372-s4-pairwise
Draft

dmihalcik-virtru wants to merge 1 commit into
DSPX-4372-s3-multi-armfrom
DSPX-4372-s4-pairwise

Conversation

@dmihalcik-virtru

Copy link
Copy Markdown
Member

Stage 4 of the six-stage plan. Stacked on #658.

What it does

When a run has more than one candidate, every pair of candidates is compared directly. These comparisons never fail the build.

  • Verdicts: FASTER / SLOWER / TIED / INCONCLUSIVE, corrected in their own BH family so they cost the regression gate no power.
  • Tails at α/2: neither candidate is privileged, so the question is two-sided. feat(xtest): compare up to four benchmark arms in one run #621 tested each tail at α, which doubles the chance of ranking two equal builds.
  • TIED requires the interval to lie inside the ±threshold band, which is TOST at 2.5%.
  • Order doesn't matter: swapping a pair turns FASTER into SLOWER, so the order candidates are listed in can't change a conclusion.
  • Winner rules: perf/conclusions.py decides winners once, and the JSON (conclusions), terminal and markdown all read that result. A winner must be FASTER than every other candidate, and a tripped A/A control suppresses the winner however narrow the interval. Ties and unresolved competitors say which pair blocked the winner.
  • Control: the control now yields every copy-to-copy pair, so the noise floor covers the widest spacing between any two candidates.

This fixes the remaining two #621 review findings: a winner named from the top two point estimates, and a tripped control not suppressing the winner.

Compatibility

  • Additive to the stage-3 schema: candidate pairs appear under contrasts, and there's a new top-level conclusions.
  • Two-arm runs are unchanged: there's nothing to compare, and conclusions is empty.

Validation

  • 278 passed. ruff, ruff format and pyright are clean.
  • New tests:
    • Reversing a comparison preserves the conclusion.
    • Candidate contrasts don't change the gate's adjusted p-values.
    • A clear two-candidate difference names a winner without failing the build.
    • A tie names no winner.
    • At three candidates, a resolved first pair plus an inconclusive third competitor names no winner.
    • A tripped control suppresses the winner.
    • With no control, everything is unresolved.
    • Each tail is tested at α/2.

Stage 4 of the DSPX-4372-t1 plan. With more than one candidate, every pair of
candidates is compared directly. These comparisons never gate.

- perf/stats.py: FASTER/SLOWER/TIED verdicts in their own BH family. Each
  tail is tested at alpha/2, since neither candidate is privileged; TIED
  requires the interval inside the +/-threshold band. The rule is
  antisymmetric, so listing order does not change a conclusion.
- perf/runner.py: candidate pairs follow the reference pairs; the control
  yields every copy-to-copy pair so its floor covers the widest spacing.
- perf/conclusions.py (new): pairwise conclusions per cell and metric, and
  winner eligibility, decided once. A winner must be FASTER than every
  other candidate, and a tripped A/A control suppresses it however narrow
  the interval. Ties and unresolved competitors say so.
- report JSON 'conclusions', a candidate-comparison section in the
  markdown, and the same conclusion lines on the terminal.

Fixes the three defects review found in #621: the control-registration
order (kept from stage 3), a winner named from the top two point estimates,
and a tripped control that did not suppress the winner.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
@sonarqubecloud

Copy link
Copy Markdown

@coderabbitai

coderabbitai Bot commented Oct 19, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant