feat(benchmarks): log₂ sweep colouring + --clip colour clamp#30
Merged
Conversation
…ic p95) The sweep heatmap coloured by raw ratio on plotly's linear scale, so a 2x and its mirror 1/2x looked asymmetric. Colour by log2(ratio) instead — folds symmetric around 1x, with a fold-change colourbar (1/8x...8x). Add --clip to override the colour clamp (a fold-change >1 for sweep, an absolute delta for scatter) over a new shared _symmetric_clip(magnitudes, override) helper that defaults to the symmetric p95 of the data, reused by both views. numpy promoted to a module-level import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
59387f9 to
c319c92
Compare
Merging this PR will not alter performance
Comparing Footnotes
|
…ur in compare Same fix as the sweep view, applied across the others: - scatter: the ratio y-axis was linear, so a 2x and its mirror 1/2x read asymmetrically (they even centred it on 1.0 *linearly*). Make it log_y so folds are symmetric about 1.0; window symmetric in log space; drop non-positive ratios (a log axis can't show a 0). - compare: clamp the bar colour with the shared symmetric p95 (consistency; the bar *length* still shows the full delta). - scaling already log-scales size; left as is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dimension Was inconsistent: a fold for sweep but an absolute Δ for scatter, and compare ignored it. Now --clip is always a fold-change (>1, default symmetric p95) that bounds the *ratio* dimension wherever a view has one: - sweep: the ratio colour (±log2) - scatter: the ratio y-axis ([1/clip, clip]) — moved off the colour, which reverts to the auto symmetric-p95 Δ clamp - compare / scaling: no ratio axis → ignored Validation is now uniform (fold-change > 1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ear per plot) Colour is the one thing you can't adjust after the plot is drawn (axes zoom). So --clip targets the colour only, and its unit follows the plot's colour scale: - sweep (colour = log2 ratio): a fold-change (>1) - scatter / compare (colour = absolute Δ): a linear Δ bound - scaling: no diverging colour → ignored Default stays the symmetric p95. Axes are full-range and zoomable — scatter's y-axis no longer p95-clips (which hid outlier points). Validation is per-scale (fold > 1 for sweep; any positive for the linear ones). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…for inputs
- compare: --sort was misleading — bars were hardcoded alphabetical
(sort_values('test_id')) while --sort only switched the dimension. Now it
sorts by the chosen Δ (delta_abs/delta_pct): biggest regressions on top,
improvements at the bottom. The name/help are finally truthful.
- plot --order {input,version}: default 'input' preserves the order you pass
(the plot never re-sorts); 'version' sorts inputs by parsed linopy-<ver>,
fixing a glob's string order (0.3.10 before 0.3.2) for release-history sweeps.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Applied after --order, so e.g. --order version --reverse = newest-first (which also makes the newest snapshot the sweep baseline / compare 'a'). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
plot serves arbitrary snapshots (bench labels, baseline.json, …), so parsing linopy-<ver> from filenames is a leaky abstraction — --order version is meaningless for non-version snapshots. plot already preserves input order, so callers control the axis by the order they pass. The --sort fix (compare bars sort by Δ, not alphabetically) stays — that was a real bug. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TODO (human): one or two sentences on why — e.g. "the sweep heatmap mis-rendered ratios on a linear scale; this fixes it and adds a colour clamp for the release-history memory sweep."
Note
The following was generated by AI (Claude).
What
log₂(ratio)instead of raw ratio. Plotly's continuous colour scale is linear with no log mode, so the old colouring made a 2× look twice as intense as its mirror ½×. Folds are now symmetric around 1×.2×,½×, …) generated frombound(not hardcoded); cells + hover also show the×fold.--clipclamps the colour scale — the one thing you can't zoom after rendering (default symmetric p95). Unit follows the plot's colour: a fold-change (>1) for fold-coloured sweep (--clip 8= ⅛×–8×), an absolute Δ for Δ-coloured scatter/compare. scaling has no diverging colour and ignores it. Axes stay full-range and zoomable._symmetric_clip(magnitudes, override)helper, reused by both scatter and sweep.--sortnow actually sorts the compare bars by the chosen Δ — it was hardcoded alphabetical while--sortonly switched the dimension (the name/help were misleading). Biggest regressions on top, improvements at the bottom.plotpreserves input order — you set the axis order by the order you pass snapshots (no ordering flags;plotserves arbitrary snapshots, not justlinopy-<ver>).plot_scatter).Verify
ruff+mypyclean.--clipoverrides (the colourbar range/ticks followbound).--clipvalidation is contextual: positive always;>1only required for the sweep (fold-change) view.Motivation
Surfaced while building a release-history memory sweep (
memory sweepacross linopy 0.2→0.7, plotted with--view sweep): a couple of 10× outliers washed the heatmap to white, and the linear colour scale skewed folds.log₂+ the p95/--clipclamp make it legible.