Skip to content

feat(xtest): compare up to four benchmark arms in one run - #621

Closed
dmihalcik-virtru wants to merge 2 commits into
mainfrom
DSPX-4372-02-karm-core
Closed

dmihalcik-virtru wants to merge 2 commits into
mainfrom
DSPX-4372-02-karm-core

Conversation

@dmihalcik-virtru

Copy link
Copy Markdown
Member

Split out of #583. Stacked on #620.

Generalizes the paired A/B benchmark from a fixed baseline/candidate pair to 2-4 named arms, so a bake-off between competing implementations of the same feature is one measurement on one runner instead of two separate two-arm dispatches whose ratios share no denominator.

  • perf/runner.py: Arm/CellResult keyed by arm_ids and an explicit reference, replacing the baseline/candidate pair. The A/A control now runs K copies of the reference (not a cheap pair), since the last arm in a K-arm round carries more within-round drift than an adjacent pair does.
  • perf/stats.py: every non-reference arm gets its own gated contrast against the reference; every pair of non-reference arms gets a symmetric FASTER/SLOWER/TIED head-to-head that never gates the build. Three BH-corrected families (gated / symmetric / ungated) so a bake-off costs the regression gate no power.
  • perf/report.py, fixtures/bench.py, conftest.py: --bench-refs (with --bench-baseline/--bench-candidate kept as two-arm shorthand), arm resolution and comparability checks generalized to K arms, and bake-off results rendered in the job summary and terminal output.
  • xtest.yml: bench-refs/bench-budget-seconds/bench-max-rounds inputs, a validate-and-normalize step, and a per-arm resolution step in the bench job. Budget defaults scale by arm-count/2 since a K-arm round costs K invocations, and the job timeout goes to 240m to cover it.

Depends on #620 (otdf-sdk-mgr slash-flattening): named-ref arms can be arbitrary branches, and an unflattened tag breaks build discovery.

A branch ref like 'feat/DSPX-2604-createtdf-chunked' resolved by name (not
by SHA) kept its slash in the tag, nesting dist/<tag>/ and src/<tag>/ one
level deeper than every consumer expects: xtest's all_versions_of() lists
dist/*/ and the Go Makefile finds src/*/, so the build was silently
discovered as a bare 'feat' directory with no cli.sh in it.

Flatten the tag the same way _classify_sha_match already flattens a branch
reached by SHA.
Generalizes the paired A/B benchmark from a fixed baseline/candidate pair to
2-4 named arms, so a bake-off between competing implementations of the same
feature is one measurement on one runner instead of two separate two-arm
dispatches whose ratios share no denominator.

- perf/runner.py: Arm/CellResult keyed by arm_ids and an explicit reference,
  replacing the baseline/candidate pair. The A/A control now runs K copies of
  the reference (not a cheap pair), since the last arm in a K-arm round
  carries more within-round drift than an adjacent pair does. Also folds in
  trunk's independent refinements: per-arm deadline capping so one slow arm
  can't run past the shared budget, and a strict configured-minimum-rounds
  check (previously only the statistical hard floor was enforced).
- perf/stats.py: every non-reference arm gets its own gated contrast against
  the reference; every pair of non-reference arms gets a symmetric
  FASTER/SLOWER/TIED head-to-head that never gates the build. Three
  BH-corrected families (gated / symmetric / ungated) so a bake-off costs the
  regression gate no power. Also folds in trunk's has_candidate_comparisons
  fix, which catches a control-only run that trunk's simpler nothing_measured
  check would have missed.
- perf/report.py, fixtures/bench.py, conftest.py: --bench-refs (with
  --bench-baseline/--bench-candidate kept as two-arm shorthand), arm
  resolution and comparability checks generalized to K arms, and bake-off
  results rendered in the job summary and terminal output.
- xtest.yml: bench-refs/bench-budget-seconds/bench-max-rounds inputs, a
  validate-and-normalize step, and a per-arm resolution step in the bench
  job. Budget defaults scale by arm-count/2 since a K-arm round costs K
  invocations, and the job timeout goes to 240m to cover it.

Depends on the otdf-sdk-mgr slash-flattening fix: named-ref arms can be
arbitrary branches, and an unflattened tag breaks build discovery.
@coderabbitai

coderabbitai Bot commented Sep 23, 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

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.

@sonarqubecloud

Copy link
Copy Markdown

Base automatically changed from DSPX-4372-01-resolve-slash-flatten to main September 28, 2026 14:37
@dmihalcik-virtru

Copy link
Copy Markdown
Member Author

Superseded, not abandoned. This draft does six unrelated things in 3211 lines — fixes a statistical bug, converts two arms to K, adds head-to-head conclusions, rewrites the report, and reimplements benchmark input parsing in workflow shell — and is not reviewable as one unit.

Replaced by the six-stage stack described in spec/DSPX-4372-t1.md, starting with #625. Review found three defects here that the split fixes rather than carries forward: the control-key registration was moved after the RSS-censoring continue in runner.analyze (a censored control-only run then exits green claiming it measured something) and both tests that would have caught it were deleted; bake_offs names a winner from point estimates alone after reading one of three contrasts at four arms; and a tripped noise control does not suppress the winner sentence.

The DSPX-4372-02-karm-core branch is deliberately left in place — #622 and #623 are still based on it.

@dmihalcik-virtru

Copy link
Copy Markdown
Member Author

Stack progress: #625 (stage 1, statistical fixes, CI green) and #626 (stage 2, shared local benchmark preparation, stacked on #625) are open. Stages 3–6 — multi-arm measurement, pairwise conclusions, thin CI integration, report presentation — follow as their dependencies land.

One consequence worth stating plainly for #622 and #623: their base branch DSPX-4372-02-karm-core will now never merge, so they are dangling until someone retargets them.

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