Skip to content

frame-benchmarking: allow 2-point slope fits in min_squares_iqr#12075

Merged
gui1117 merged 5 commits into
masterfrom
fix/benchmarking-min-squares-2-point-slope
May 20, 2026
Merged

frame-benchmarking: allow 2-point slope fits in min_squares_iqr#12075
gui1117 merged 5 commits into
masterfrom
fix/benchmarking-min-squares-2-point-slope

Conversation

@tindzk

@tindzk tindzk commented May 14, 2026

Copy link
Copy Markdown
Contributor

Description

a33b7c2 short-circuits min_squares_iqr to median_value whenever r.len() <= 2 to avoid OLS panics on --steps=1 --repeats=1. That fallback also catches the case where two samples sit at different x-values, where the slope is exactly determined. However, benchmarks whose valid sample set is narrowed to two values (e.g. by BenchmarkError::Skip filtering a Linear<lo, hi> parameter) lose their linear component.

Route the r.len() <= 2 fallback through median_slopes instead, which fits a slope exactly from two samples at different x-values. The degenerate "all samples share one x" case now surfaces an explicit error from median_slopes's own check.

Integration

No API or storage changes. Downstream runtimes do not need to update code.

Regenerating weights may produce different values for benchmarks whose valid sample set is exactly 2 points at distinct x-values. The resulting weight expression will now include a linear component fitted via median_slopes instead of falling back to the constant median.

Benchmarks whose 2 remaining samples share an x-value will now fail with an explicit median_slopes error rather than returning the median. If you see this, broaden the parameter range or increase --steps/--repeats so more than one distinct x-value is sampled.

Review Notes

min_squares_iqr had a single guard combining two unrelated short-circuits:

if r[0].components.is_empty() || r.len() <= 2 {
    return Self::median_value(r, selector);
}

The components.is_empty() branch is correct: a benchmark with no parameters has no slope to fit, so a constant median should be returned. The r.len() <= 2 branch was a workaround for linregress's OLS fit, which is under-determined with two samples and one intercept + one slope variable. Both cases fell through to the same median_value call, which drops slopes entirely.

The no-components fallback is kept on median_value, while r.len() <= 2 is routed to median_slopes instead. median_slopes is well-suited: it forms the pairwise slope list (y_i - y_j) / (x_i - x_j) over samples with distinct x-values, takes the median, then derives the intercept from per-sample offsets. With exactly two distinct-x samples the slope list has length 1, so the median is the exact slope. No regression is needed.

`a33b7c2e36` short-circuits `min_squares_iqr` to `median_value` whenever
`r.len() <= 2` to avoid OLS panics on `--steps=1 --repeats=1`. That
fallback also catches the case where two samples sit at different
x-values, where the slope is exactly determined. However, benchmarks
whose valid sample set is narrowed to two values (e.g. by
`BenchmarkError::Skip` filtering a `Linear<lo, hi>` parameter)
lose their linear component.

Route the `r.len() <= 2` fallback through `median_slopes` instead,
which fits a slope exactly from two samples at different x-values. The
degenerate "all samples share one x" case now surfaces an explicit error
from `median_slopes`'s own check.
@tindzk tindzk added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label May 18, 2026
@tindzk

tindzk commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

/cmd prdoc

@tindzk tindzk marked this pull request as ready for review May 18, 2026 06:50
@tindzk tindzk requested a review from a team as a code owner May 18, 2026 06:50
@tindzk tindzk requested review from gui1117 and shawntabrizi May 18, 2026 06:54
Comment thread prdoc/pr_12075.prdoc Outdated
@gui1117 gui1117 enabled auto-merge May 20, 2026 06:31
@gui1117 gui1117 added the T12-benchmarks This PR/Issue is related to benchmarking and weights. label May 20, 2026
@gui1117 gui1117 added this pull request to the merge queue May 20, 2026
Merged via the queue into master with commit 2f6033f May 20, 2026
287 of 288 checks passed
@gui1117 gui1117 deleted the fix/benchmarking-min-squares-2-point-slope branch May 20, 2026 08:30
@EgorPopelyaev EgorPopelyaev added the A4-backport-unstable2604 Pull request must be backported to the unstable2604 release branch label May 21, 2026
@paritytech-release-backport-bot

Copy link
Copy Markdown

Successfully created backport PR for unstable2604:

EgorPopelyaev added a commit that referenced this pull request May 21, 2026
Backport #12075 into `unstable2604` from tindzk.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Co-authored-by: Tim Nieradzik <tim@sparse.tech>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Egor_P <egor@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A4-backport-unstable2604 Pull request must be backported to the unstable2604 release branch T1-FRAME This PR/Issue is related to core FRAME, the framework. T12-benchmarks This PR/Issue is related to benchmarking and weights.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants