From 63a998a3cd24ea29f3b68cafd4d9b56ec23ea994 Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Sat, 12 Sep 2026 20:15:44 -0500 Subject: [PATCH 1/2] Put the benchmark speedup table on the run summary page --- .github/workflows/bench.yml | 3 +++ toolchain/mfc/bench.py | 39 +++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 473f0988c..235228c92 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -152,6 +152,9 @@ jobs: - name: Generate & Post Comment if: always() + env: + # Names the matrix leg in the run-summary table; every leg writes its own. + MFC_BENCH_SUMMARY_LABEL: ${{ matrix.cluster }} ${{ matrix.device }} ${{ matrix.interface }} run: | (cd pr && . ./mfc.sh load -c ${{ matrix.flag }} -m g) (cd pr && ./mfc.sh bench_diff ../master/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml ../pr/bench-${{ matrix.device }}-${{ matrix.interface }}.yaml) diff --git a/toolchain/mfc/bench.py b/toolchain/mfc/bench.py index daf3cb501..0b1fad420 100644 --- a/toolchain/mfc/bench.py +++ b/toolchain/mfc/bench.py @@ -193,6 +193,39 @@ def bench(targets=None): cons.unindent() +def _write_step_summary(lhs_path: str, rhs_path: str, rows: list, warnings: list): + """Put the speedup table on the workflow run's summary page. + + The same numbers already go to stdout, but reading them there means expanding the + right step of the right matrix leg. GitHub renders $GITHUB_STEP_SUMMARY inline on + the job, so the table is visible without opening anything. Does nothing outside + Actions, where the variable is unset. + """ + path = os.environ.get("GITHUB_STEP_SUMMARY") + if not path or not rows: + return + + # The matrix leg is not otherwise on the page, and every leg writes its own summary. + leg = os.environ.get("MFC_BENCH_SUMMARY_LABEL", "") + heading = f"### Benchmark: {leg}" if leg else "### Benchmark" + + lines = [ + heading, + "", + f"Speedups from `{lhs_path}` to `{rhs_path}`; greater than 1 is faster.", + "", + "| Case | Pre Process | Simulation | Post Process |", + "| --- | --- | --- | --- |", + ] + lines += [f"| `{slug}` | {pre} | {sim} | {post} |" for slug, pre, sim, post in rows] + if warnings: + lines += ["", "**Below threshold**", ""] + [f"- {w}" for w in warnings] + lines.append("") + + with open(path, "a", encoding="utf-8") as f: + f.write("\n".join(lines)) + + def diff(): lhs, rhs = file_load_yaml(ARG("lhs")), file_load_yaml(ARG("rhs")) lhs_path = os.path.relpath(ARG("lhs")) @@ -233,6 +266,8 @@ def _lock_to_str(lock): table.add_column("[bold]Post Process[/bold]", justify="right") err = 0 + summary_rows = [] + warnings = [] for slug in slugs: lhs_summary, rhs_summary = lhs["cases"][slug]["output_summary"], rhs["cases"][slug]["output_summary"] speedups = ["N/A", "N/A", "N/A"] @@ -250,6 +285,7 @@ def _lock_to_str(lock): exec_time_value = lhs_summary[target.name]["exec"] / rhs_summary[target.name]["exec"] if exec_time_value < 0.9: cons.print(f"[bold yellow]Warning[/bold yellow]: Exec time speedup for {target.name} is less than 0.9 - Case: {slug}") + warnings.append(f"exec speedup {exec_time_value:.2f} < 0.90 for {target.name} in `{slug}`") speedups[i] = f"Exec: {exec_time_value:.2f}" if target == SIMULATION: if not math.isfinite(lhs_summary[target.name]["grind"]) or not math.isfinite(rhs_summary[target.name]["grind"]): @@ -260,12 +296,15 @@ def _lock_to_str(lock): speedups[i] += f" & Grind: {grind_time_value:.2f}" if grind_time_value < 0.95: cons.print(f"[bold yellow]Warning[/bold yellow]: Grind time speedup for {target.name} below threshold (<0.95) - Case: {slug}") + warnings.append(f"grind speedup {grind_time_value:.2f} < 0.95 for {target.name} in `{slug}`") except Exception as e: cons.print(f"[bold red]ERROR[/bold red]: Failed to compute speedup for {target.name} in {slug}: {e}\n{traceback.format_exc()}") err = 1 table.add_row(f"[magenta]{slug}[/magenta]", *speedups) + summary_rows.append((slug, *speedups)) cons.raw.print(table) + _write_step_summary(lhs_path, rhs_path, summary_rows, warnings) if err: raise MFCException("Benchmarking failed") From 1abdfd121c04ef41d53e6c8caaff961f2948b021 Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Sat, 12 Sep 2026 20:36:56 -0500 Subject: [PATCH 2/2] Keep a failed summary write from failing the benchmark --- toolchain/mfc/bench.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/toolchain/mfc/bench.py b/toolchain/mfc/bench.py index 0b1fad420..85a182742 100644 --- a/toolchain/mfc/bench.py +++ b/toolchain/mfc/bench.py @@ -193,7 +193,7 @@ def bench(targets=None): cons.unindent() -def _write_step_summary(lhs_path: str, rhs_path: str, rows: list, warnings: list): +def _write_step_summary(lhs_path: str, rhs_path: str, rows: typing.List[typing.Tuple[str, str, str, str]], warnings: typing.List[str]): """Put the speedup table on the workflow run's summary page. The same numbers already go to stdout, but reading them there means expanding the @@ -222,8 +222,14 @@ def _write_step_summary(lhs_path: str, rhs_path: str, rows: list, warnings: list lines += ["", "**Below threshold**", ""] + [f"- {w}" for w in warnings] lines.append("") - with open(path, "a", encoding="utf-8") as f: - f.write("\n".join(lines)) + # The summary is a convenience on top of output that already went to stdout, so a filesystem + # problem here must not fail a benchmark that otherwise succeeded. Narrow to OSError: anything + # else is a bug in the lines above and should surface. + try: + with open(path, "a", encoding="utf-8") as f: + f.write("\n".join(lines)) + except OSError as exc: + cons.print(f"[bold yellow]Warning[/bold yellow]: could not write the benchmark step summary: {exc}") def diff():