Add combined results and plot generation by avolkov-intel · Pull Request #219 · IntelPython/scikit-learn_bench

avolkov-intel · 2026-08-17T10:58:23Z

Description

New CLI options

--combined-results — builds two extra sheets:
- All cases — all algorithm results on one page (group name, sklearn time[ms], sklearnex time[ms], speedup, then parameters). KNN is split into
  brute/kd_tree; rows sorted by dtype then dataset; each group ends with per-dtype and total GEOMEAN formula rows with a per-group speedup color scale.
- Summary (for plots) — one row per algorithm referencing the group GEOMEAN cells (total + fp32 + fp64), split into Training/Inference sections with
  section and total geomeans.
--draw-plots / --plot-output <path> — renders a Training/Inference speedup bar chart (geomean per algorithm, combining fp32 & fp64) with
Intel-branded titles and disclaimer footnote.

Other changes

--performance-stability-metrics now additionally emits a median time[ms] column (alongside existing 1st run time[ms], time CV), included in the
individual sheets and "All cases".

Use --compatibility-mode together with --combined-results so sklearn/sklearnex rows merge correctly.

Completeness and readability

I have commented my code, particularly in hard-to-understand areas.
I have updated the documentation to reflect the changes or created a separate PR with updates and provided its number in the description, if necessary.
Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
I have resolved any merge conflicts that might occur with the base branch.

Testing

I have run it locally and tested the changes extensively.
All CI jobs are green or I have provided justification why they aren't.
I have extended testing suite if new functionality was introduced in this PR.

david-cortes-intel · 2026-08-17T11:02:59Z

@avolkov-intel Please document these options in the .md files where the other options are described.

avolkov-intel · 2026-08-17T11:13:00Z

@avolkov-intel Please document these options in the .md files where the other options are described.

Done

avolkov-intel · 2026-08-17T13:00:11Z

CI failures are not related to this PR

ethanglaser · 2026-08-17T17:25:33Z

+    if (
+        "n_jobs" in results.columns
+        and results["n_jobs"].isna().any()
+        and results["n_jobs"].notna().any()


Would it make more sense to drop and results["n_jobs"].notna().any() and just drop n_jobs col if any are na? Or what is the reason for this change

If we compare sklearn algorithm with sklearnex counterpart, n_jobs parameter might only be present in sklearnex but not in sklearn. In this case the measurements will not be compared and report & plots would not be generated. So the idea of this change if both not nans and nans are present we can drop this column so that reports are properly generated. If all values are na there's no need to drop the column

This feature should only be used for comparison of sklearn vs sklearnex btw

ethanglaser · 2026-08-17T17:35:05Z

+    Each block ends with per-dtype and total GEOMEAN formula rows plus a speedup color scale.
+    Returns list of (group_name, total_geomean_row, {dtype: geomean_row}) for the summary sheet.
+    """
+    KNN = ("KNeighborsClassifier", "KNeighborsRegressor")


Is there a way to make this knn-specific logic more flexible? ie right now we have 2 different knn methods but what it we wanted to use 2 different LogReg solvers - would we need to keep adding in algo-specific logic or can we set it up to be more general?

Potentially yes, but this would make the code more complex and we don't need it right now

david-cortes-intel · 2026-08-18T12:43:43Z

+| `--performance-stability-metrics`</br>`-psm`   |        | False                                               |                                                | Adds performance stability metrics (`1st run time[ms]`, `1st-mean run ratio`, `median time[ms]`, `time CV`) to the report.       |
+| `--combined-results`                           |        | False                                               |                                                | [EXPERIMENTAL] Creates `All cases` and `Summary (for plots)` sheets combining time and speedup of all algorithms with per-dtype/total GEOMEAN rows (use with `--compatibility-mode`). |
+| `--draw-plots`                                 |        | False                                               |                                                | [EXPERIMENTAL] Draws Training/Inference speedup bar charts from the combined results (requires `--combined-results`).             |
+| `--plot-output`                                | str    | None                                                |                                                | [EXPERIMENTAL] Output file path for plots (e.g. `plots.png`); if unset, plots are shown interactively.                           |


Does this require passing --draw-plots?

Well technically no, you can provide it without draw plots it would be just ignored

Please add that kind of detail in these docs.

david-cortes-intel · 2026-08-18T13:37:00Z

@avolkov-intel Would this work when comparing against cuml? If so, how would it handle cases where parameters do not match?

david-cortes-intel · 2026-08-18T13:38:31Z

 | `--compatibility-mode`                         |        | False                                               |                                                | [EXPERIMENTAL] Compatibility mode drops and modifies results to make them comparable (for example, sklearn and cuML parameters). |
+| `--performance-stability-metrics`</br>`-psm`   |        | False                                               |                                                | Adds performance stability metrics (`1st run time[ms]`, `1st-mean run ratio`, `median time[ms]`, `time CV`) to the report.       |
+| `--combined-results`                           |        | False                                               |                                                | [EXPERIMENTAL] Creates `All cases` and `Summary (for plots)` sheets combining time and speedup of all algorithms with per-dtype/total GEOMEAN rows (use with `--compatibility-mode`). |
+| `--draw-plots`                                 |        | False                                               |                                                | [EXPERIMENTAL] Draws Training/Inference speedup bar charts from the combined results (requires `--combined-results`).             |


This option without --plot-output doesn't appear to do anything. It doesn't show anything interactive for me, or at least not when used in an SSH session.

david-cortes-intel · 2026-08-18T13:39:51Z

@avolkov-intel Please add information about how to generate a publication-grade plot, with details in the bottom text filled in.

david-cortes-intel · 2026-08-18T13:41:08Z

+    ws = wb.create_sheet(title="All cases", index=1)
+    row, geomean_rows = 1, []
+
+    def geomean_row(label, r0, r1):


Please make row a parameter.

david-cortes-intel · 2026-08-18T13:41:59Z

+    return df
+
+
+def write_all_cases_2_sheet(dfs, wb):


Please add a description of what wb is. Better yet if you can add type hints.

david-cortes-intel · 2026-08-18T13:42:20Z

+    Each block ends with per-dtype and total GEOMEAN formula rows plus a speedup color scale.
+    Returns list of (group_name, total_geomean_row, {dtype: geomean_row}) for the summary sheet.
+    """
+    KNN = ("KNeighborsClassifier", "KNeighborsRegressor")


What about LocalOutlierFactor and NearestNeighbors?

david-cortes-intel · 2026-08-18T13:43:22Z

+                if c in gdf.columns
+            ]
+            if sort_cols:
+                gdf = gdf.sort_values(sort_cols, kind="mergesort")


Is this trying to do a stable sort? Otherwise, why pass the parameter?

david-cortes-intel · 2026-08-18T13:44:46Z

+                        j = i
+                        while j + 1 < len(dtypes) and dtypes[j + 1] == dtypes[i]:
+                            j += 1
+                        dtype_rows[dtypes[i]] = geomean_row(


Since this one doesn't embed geomean as a formula, maybe the code could be a lot more readable if this were to pre-generate subtables as dataframes and store them in a list to write at the end.

david-cortes-intel · 2026-08-18T13:45:30Z

+        return [f"={src_sheet_name}!{c}{row_num}" for c in "BCD"]
+
+    def write_section(title, rows):
+        nonlocal current_row


Please switch it to parameter.

david-cortes-intel · 2026-08-18T13:46:45Z

                new_ws.append([None])


+def draw_summary_plots(all_cases_df: pd.DataFrame, output_file: str = None):


Would be helpful to have a sub-function to generate the plot returning matplotlib objects (plt, fig, axes).

avolkov-intel requested a review from david-cortes-intel as a code owner August 17, 2026 10:58

avolkov-intel added 3 commits August 17, 2026 13:00

Add an option --combined-results to create list with all use cases

49bd845

Add options to draw comparison plots

356dc0a

Handle n_jobs param correctly and add Summary 2 and All cases 2 pages

24ab534

avolkov-intel added 3 commits August 17, 2026 13:03

Add footnote for plot and add metrics

e842a63

Update footnote

98fa891

Update docs

1886459

avolkov-intel force-pushed the dev/anatolyv-combined-results branch from ceda047 to 1886459 Compare August 17, 2026 11:11

Code format

59276a2

ethanglaser reviewed Aug 17, 2026

View reviewed changes

david-cortes-intel reviewed Aug 18, 2026

View reviewed changes

Minor docs update

2f671bd

david-cortes-intel reviewed Aug 18, 2026

View reviewed changes

		new_ws.append([None])


		def draw_summary_plots(all_cases_df: pd.DataFrame, output_file: str = None):

Uh oh!

Conversation

avolkov-intel commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

New CLI options

Other changes

Uh oh!

david-cortes-intel commented Aug 17, 2026

Uh oh!

avolkov-intel commented Aug 17, 2026

Uh oh!

avolkov-intel commented Aug 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avolkov-intel Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-cortes-intel commented Aug 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-cortes-intel commented Aug 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

avolkov-intel commented Aug 17, 2026 •

edited

Loading

avolkov-intel Aug 18, 2026 •

edited

Loading