From 05e475afd0a2242923e6091efddd930d0717c731 Mon Sep 17 00:00:00 2001 From: Muhammad Haseeb <14217455+mhaseeb123@users.noreply.github.com> Date: Sat, 30 May 2026 00:19:02 +0000 Subject: [PATCH 1/6] Skill to compare performance of a branch or PR with main --- .agents/skills/perf-compare-cudf/SKILL.md | 215 +++++++++++ .../perf-compare-cudf/scripts/compare.py | 352 ++++++++++++++++++ 2 files changed, 567 insertions(+) create mode 100644 .agents/skills/perf-compare-cudf/SKILL.md create mode 100644 .agents/skills/perf-compare-cudf/scripts/compare.py diff --git a/.agents/skills/perf-compare-cudf/SKILL.md b/.agents/skills/perf-compare-cudf/SKILL.md new file mode 100644 index 000000000000..e182601cec61 --- /dev/null +++ b/.agents/skills/perf-compare-cudf/SKILL.md @@ -0,0 +1,215 @@ +--- +name: perf-compare-cudf +description: Use when the user invokes /perf-compare-cudf, asks to benchmark a cudf branch or PR against main, or wants a performance comparison for libcudf changes. +--- + +Use this skill when the user invokes `/perf-compare-cudf` with either: +- **nothing / "current branch"** (default) — benchmark the currently checked-out branch, or +- **a cudf PR link / number** (e.g. `https://github.com/rapidsai/cudf/pull/12345`) — check out that PR first. + +# Goal +Run a curated subset of nvbench benchmarks on the PR branch, then on a branch in sync with `/main`, and produce a markdown report comparing the two with significant changes highlighted. + +Note the cudf GH repository link: https://github.com/rapidsai/cudf +`` = the git remote pointing at cudf GH repo (often `upstream`). Detect it with `git remote -v`. + +--- + +## Workflow checklist + +Copy and track progress: + +``` +- [ ] 0. Confirm devcontainer + env (/build-test-cudf skill) +- [ ] 1. Determine target + which case applies (A: target is current branch / B: target is a different PR branch) +- [ ] 2. Get onto the PR side per the case (Case A: stay put, keep local changes; Case B: record start branch, stash, checkout clean PR branch) +- [ ] 3. Decide which benchmarks to run (infer from diff; confirm with user) +- [ ] 4. Decide axis coverage (full vs skim); confirm with user +- [ ] 5. Pick an idle GPU (nvidia-smi) +- [ ] 6. Build libcudf with -DBUILD_BENCHMARKS=ON on PR side +- [ ] 7. Run benchmarks on PR side -> results/pr/ +- [ ] 8. Stash now (Case A only — preserve local changes), then switch to a branch in sync with latest /main +- [ ] 9. Rebuild libcudf with benchmarks on main +- [ ] 10. Run the SAME benchmark invocations on main -> results/main/ +- [ ] 11. Generate comparison report +- [ ] 12. Restore: return to the starting branch, unstash; clean up any temp branch created +``` + +--- + +## Step 0: Devcontainer + build environment + +Read and follow `/build-test-cudf` (skill at `.agents/skills/build-test-cudf/SKILL.md`) to: +- Confirm we are in a cudf devcontainer (username `coder`). If not, stop. +- Get and follow instructions to build (and troubleshoot builds) libcudf with specified CMake options + +## Step 1: Determine target and which case applies + +Record the starting branch (`git branch --show-current`). Then classify into one of two cases — this decides how local changes and stashing are handled: + +- **Case A — target is the current branch.** Applies when no PR was given, OR a PR was given and the current branch already is the PR's branch (tracks it). **Keep local uncommitted changes as-is; do NOT stash yet.** We build & benchmark the PR side first, then stash only when leaving for main. +- **Case B — target is a PR on a different branch.** Applies when a PR was given and the current branch is NOT the PR's branch. + +For a PR, resolve its head branch (to compare against current and to check out in Case B). Use `gh` if available: + +```bash +gh pr view --repo rapidsai/cudf --json headRefName,headRepository,headRepositoryOwner,baseRefName,title +``` + +If `gh` auth is unavailable, fetch the PR ref directly: + +```bash +git fetch pull//head:pr- +``` + +## Step 2: Get onto the PR side + +- **Case A:** stay on the current branch. Do not stash and do not checkout — local changes stay applied through the PR-side build and benchmark run. +- **Case B:** the PR side must be clean, so: + 1. Record the starting branch and stash any uncommitted changes, **recording exactly what was stashed** so it can be restored at the very end: + + ```bash + git stash push -m "perf-compare-cudf: wip" -- # or `git stash push -m ...` for everything + git stash list + ``` + 2. Check out the PR branch: reuse an existing local branch tracking it; otherwise check out the fetched ref (`git checkout pr-`). `git checkout ` works only when a local/remote-tracking branch of that name already exists (e.g. same-repo PRs). + +## Step 3: Decide which benchmarks to run + +Infer candidate benchmarks from the diff vs `/main`: + +```bash +git fetch main +git diff --name-only /main...HEAD +``` + +Map changed source areas to benchmark binaries. Benchmarks are built at `cpp/build/latest/benchmarks`. List all with `ls cpp/build/latest/benchmarks` and inspect a binary's benchmarks/axes with ` --list`. + +Examples of mapping: +- Parquet writer changes → `PARQUET_WRITER_NVBENCH` +- Parquet reader / cuIO source changes → `PARQUET_READER_NVBENCH`, `PARQUET_READER_COMPRESSED_NVBENCH`, `PARQUET_MULTITHREAD_READER_NVBENCH`, `HYBRID_SCAN_*_NVBENCH`, etc. +- Join changes → `JOIN_NVBENCH`; sort → `SORT_NVBENCH`; strings → `STRINGS_NVBENCH`. + +**Always confirm the benchmark list with the user** (use AskQuestion if available). The user may name specific benchmarks, give a hint, or ask to run all of them. + +## Step 4: Decide axis coverage + +Each nvbench benchmark has axes with many values; full cross-product is large. Ask the user (AskQuestion) whether to: +- **Skim** (default, fast): pick a few representative values per axis — smallest, largest, and a couple in the middle. +- **Full**: run all configurations. + +Inspect axes per binary: + +```bash +cpp/build/latest/benchmarks/ --list # shows axes + values +cpp/build/latest/benchmarks/ --help-axes # axis spec syntax +``` + +Override axes with `-a name=[v1,v2,...]` (and `-b ` to scope). **Record the exact axis values chosen** and use the identical invocation on both branches. + +## Step 5: Pick an idle GPU + +```bash +nvidia-smi --query-gpu=index,name,utilization.gpu,memory.used,memory.total --format=csv +``` + +Choose one idle GPU (0% util, ~0 MiB used). Run on a single device only. Pass `CUDA_VISIBLE_DEVICES=` and also `-d 0` to every nvbench invocation (after masking, the chosen device is index 0). + +## Step 6: Build libcudf (PR side) + +Use the `/build-test-cudf` skill to configure libcudf with `-DBUILD_BENCHMARKS=ON` CMake option and build. **Do not miss** this CMake option as libcudf benchmarks will not build otherwise. In Case A this builds with the local uncommitted changes applied (intended). In case of errors, use the same skill to troubleshoot or clean, reconfigure **with the CMake option** and rebuild as needed. + +## Step 7: Run benchmarks on PR side + +Write CSV + log per benchmark binary into a results dir (suggest `.agents/benchmark-results//pr/`). For each benchmark binary: + +```bash +CUDA_VISIBLE_DEVICES= cpp/build/latest/benchmarks/ -d 0 \ + -b -a ... \ + --csv /pr/.csv 2>&1 | tee /pr/.log +``` + +Notes: +- nvbench may emit a benign segfault at the very end of a suite — **ignore end-of-suite segfaults**. +- If a config throws an exception, note it and exclude it from the comparison, but verify the **same** behavior occurs on both branches. + +## Step 8: Stash (Case A) and switch to a branch in sync with rapidsai/cudf main + +Now that PR-side data is collected, leave for the main side. The "main" side must reflect `/main` (rapidsai/cudf), not a stale fork main, and must NOT carry the PR's changes. + +1. **Case A only:** the local changes are still applied — stash them now so main is clean, **recording exactly what was stashed**: + + ```bash + git stash push -m "perf-compare-cudf: wip" -- # or `git stash push -m ...` for everything + git stash list + ``` + + (Case B already stashed in Step 2 and is on a clean PR branch.) +2. `git fetch main` +3. Decide the main-side ref: + - If local `main` already tracks `/main` and is up to date → `git checkout main`. + - If local `main` tracks a fork → pull `/main` into it, **or** create a temp branch from the remote: `git checkout -b _bench_main /main` (delete it in Step 12). +4. Do NOT apply the stash on main — main must stay clean of PR changes so the comparison is meaningful. + +## Step 9: Rebuild on main + +Follow the instructions in **Step 6: Build libcudf (PR side)** to properly configure and rebuild on the "main" branch. Note that the "main" branch must be in a clean state. + +## Step 10: Run the SAME benchmarks on main + +Use the identical `-b`/`-a` invocations and the same GPU, writing to `/main/`. + +## Step 11: Generate the comparison report + +Use the helper script (reads matching CSVs and emits markdown): + +```bash +python .agents/skills/perf-compare-cudf/scripts/compare.py \ + --pr /pr --main /main --report /COMPARISON.md +``` + +The script matches rows by `(benchmark, axis-values)`, computes `Δ = (PR - main) / main` on GPU time, and flags **significant** changes where `|Δ| >= 5%` AND `|Δ| >` max(noise of either side). It prints significant rows to stdout. + +**Re-run any flagged config once** to confirm it is not noise (especially sub-millisecond benches with high nvbench noise). If the rerun matches within noise, replace that CSV row's run and regenerate. + +See the report template at the bottom. + +## Step 12: Restore state + +- Return to the starting branch recorded in Step 1 (Case A: the original/current branch; Case B: the branch we started on before checking out the PR). +- Pop the stash to restore the user's uncommitted changes (both cases stashed something: Case A in Step 8, Case B in Step 2). +- Delete any temp branch created in Step 8 (e.g. `git branch -D _bench_main`). +- Confirm `git status` matches the pre-run state. + +--- + +## Report template + +The generated `COMPARISON.md` (and your chat summary) should include: + +```markdown +# Benchmark Comparison: /main vs PR (``) + +- GPU Time in ms. Δ = (PR - main) / main. Negative = PR faster. +- Significant: |Δ| >= 5% AND larger than max(noise) of either side. +- Hardware: , driver/CUDA if available. +- Branches: PR `` vs main ``. Axis coverage: (list values used). + +## Summary +| Benchmark Suite | # Configs | # Significant | +| ... | + +## Top N by |Δ| +| Suite / bench | axes | main (ms) | PR (ms) | Δ | noise(m/p) | +| ... | + +## Per-suite tables +（one table per benchmark, axes as columns, with FASTER/SLOWER flags） + +## Notes +- Exceptions excluded (same on both branches): ... +- End-of-suite segfaults ignored. +- Files generated: list of CSV/log paths + this report + compare.py +``` + +Always end the chat summary with: the headline (regression / improvement / within-noise), the hardware, the axis coverage, and the list of generated files. diff --git a/.agents/skills/perf-compare-cudf/scripts/compare.py b/.agents/skills/perf-compare-cudf/scripts/compare.py new file mode 100644 index 000000000000..ed1b46fbaafa --- /dev/null +++ b/.agents/skills/perf-compare-cudf/scripts/compare.py @@ -0,0 +1,352 @@ +#!/usr/bin/env python3 + +# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION. +# SPDX-License-Identifier: Apache-2.0 + +"""Compare cudf nvbench CSV results between a `main` run and a `pr` run. + +Rows are matched by (benchmark name, axis values). For each matched config we +compute the GPU-time delta (PR - main) / main and emit a markdown report. A +config is flagged "significant" when |delta| >= threshold AND |delta| exceeds +the nvbench noise on either side, so noise-dominated swings don't get flagged. + +Each input directory holds one CSV per benchmark binary, named after the binary +(as produced by ` --csv /.csv`). Only CSVs present in BOTH +directories (matched by filename) are compared. +""" + +import argparse +import csv +from dataclasses import dataclass +from pathlib import Path + +# nvbench CSV column names we read directly. +COL_BENCHMARK = "Benchmark" +COL_GPU_TIME = "GPU Time (sec)" +COL_NOISE = "Noise" +COL_SKIPPED = "Skipped" + +# Columns that are never benchmark axes (identity + the metrics we don't axis on). +NON_AXIS = { + COL_BENCHMARK, + "Device", + "Device Name", + COL_SKIPPED, + "Samples", + "CPU Time (sec)", + COL_NOISE, + COL_GPU_TIME, +} + +# nvbench emits a different set of metric/output columns per benchmark, so we +# can't enumerate them. Instead we drop any column whose name looks like a +# measured output; everything else is treated as an axis. +METRIC_HINTS = ( + "per_second", + "per_sec", + "bytes_per_second", + "BW", + "Noise", + "Samples", + "Time (sec)", + "memory", + "encoded", + "Util", + "size (bytes)", + "throughput", +) + + +def looks_like_metric(column: str) -> bool: + return any(hint.lower() in column.lower() for hint in METRIC_HINTS) + + +def pct(value: float) -> str: + return f"{value * 100:+.2f}%" + + +def axes_str(axes: dict[str, str]) -> str: + return " ".join( + f"{name}={value}" for name, value in axes.items() if value != "" + ) + + +def markdown_table(headers: list[str], rows: list[list[str]]) -> str: + """Render a GitHub-flavored markdown table from headers and string cells.""" + lines = [ + "| " + " | ".join(headers) + " |", + "|" + "|".join("---" for _ in headers) + "|", + ] + lines += ["| " + " | ".join(cells) + " |" for cells in rows] + return "\n".join(lines) + "\n" + + +@dataclass +class Comparison: + """One benchmark config measured on both branches.""" + + bench: str + axes: dict[str, str] + main_ms: float + pr_ms: float + delta: float + main_noise: float + pr_noise: float + + def significant(self, threshold: float) -> bool: + return abs(self.delta) >= threshold and abs(self.delta) > max( + self.main_noise, self.pr_noise + ) + + def flag(self, threshold: float) -> str: + if not self.significant(threshold): + return "" + return "**FASTER**" if self.delta < 0 else "**SLOWER**" + + def noise_str(self) -> str: + return f"{self.main_noise * 100:.1f}%/{self.pr_noise * 100:.1f}%" + + +@dataclass +class SuiteResult: + """Comparison outcome for a single benchmark binary.""" + + name: str + rows: list[Comparison] + only_pr: list # config keys present only in the PR run + only_main: list # config keys present only in the main run + + +def read_rows(path: Path) -> list[dict]: + with open(path) as f: + return list(csv.DictReader(f)) + + +def config_key(row: dict): + """Identity of a benchmark config: its name plus all (axis, value) pairs.""" + axes = sorted( + c for c in row if c not in NON_AXIS and not looks_like_metric(c) + ) + return (row[COL_BENCHMARK],) + tuple((axis, row[axis]) for axis in axes) + + +def compare_suite( + name: str, pr_dir: Path, main_dir: Path +) -> SuiteResult | None: + pr_csv, main_csv = pr_dir / f"{name}.csv", main_dir / f"{name}.csv" + if not pr_csv.exists() or not main_csv.exists(): + return None + + pr_by_key = {config_key(row): row for row in read_rows(pr_csv)} + main_by_key = {config_key(row): row for row in read_rows(main_csv)} + + rows: list[Comparison] = [] + for key in sorted(pr_by_key.keys() & main_by_key.keys()): + pr_row, main_row = pr_by_key[key], main_by_key[key] + if ( + pr_row.get(COL_SKIPPED) == "Yes" + or main_row.get(COL_SKIPPED) == "Yes" + ): + continue + try: + pr_time = float(pr_row[COL_GPU_TIME]) + main_time = float(main_row[COL_GPU_TIME]) + except (KeyError, ValueError): + continue + if main_time <= 0: + continue + rows.append( + Comparison( + bench=key[0], + axes={axis: value for axis, value in key[1:]}, + main_ms=main_time * 1000, + pr_ms=pr_time * 1000, + delta=(pr_time - main_time) / main_time, + main_noise=float(main_row.get(COL_NOISE) or 0), + pr_noise=float(pr_row.get(COL_NOISE) or 0), + ) + ) + + return SuiteResult( + name=name, + rows=rows, + only_pr=sorted(pr_by_key.keys() - main_by_key.keys()), + only_main=sorted(main_by_key.keys() - pr_by_key.keys()), + ) + + +def render_header(threshold: float) -> str: + return ( + "# Benchmark Comparison: main vs PR\n\n" + "- GPU Time in ms. Δ = (PR - main) / main. Negative = PR faster.\n" + f"- **Significant**: |Δ| >= {threshold * 100:.0f}% AND larger than max(noise) of either side.\n" + "- Fill in hardware (GPU/driver/CUDA), branch SHAs, and axis coverage manually.\n\n" + ) + + +def render_summary(suites: list[SuiteResult], threshold: float) -> str: + rows = [ + [ + suite.name, + str(len(suite.rows)), + str(sum(1 for r in suite.rows if r.significant(threshold))), + ] + for suite in suites + ] + return ( + "## Summary\n\n" + + markdown_table( + ["Benchmark Suite", "# Configs", "# Significant"], rows + ) + + "\n" + ) + + +def render_top(suites: list[SuiteResult], top: int) -> str: + everything = [(suite.name, row) for suite in suites for row in suite.rows] + everything.sort(key=lambda pair: abs(pair[1].delta), reverse=True) + rows = [ + [ + f"{suite_name}/{r.bench}", + axes_str(r.axes), + f"{r.main_ms:.3f}", + f"{r.pr_ms:.3f}", + pct(r.delta), + r.noise_str(), + ] + for suite_name, r in everything[:top] + ] + return ( + f"## Top {top} by |Δ|\n\n" + + markdown_table( + [ + "Suite / bench", + "axes", + "main (ms)", + "PR (ms)", + "Δ", + "noise(m/p)", + ], + rows, + ) + + "\n" + ) + + +def render_suite(suite: SuiteResult, threshold: float) -> str: + out = [f"## {suite.name}\n\n"] + if suite.only_pr or suite.only_main: + out.append( + f"_only-on-PR configs: {len(suite.only_pr)}, " + f"only-on-main: {len(suite.only_main)}_\n\n" + ) + + by_bench: dict[str, list[Comparison]] = {} + for row in suite.rows: + by_bench.setdefault(row.bench, []).append(row) + + for bench, rows in by_bench.items(): + axis_keys = sorted({axis for r in rows for axis in r.axes}) + headers = axis_keys + [ + "main (ms)", + "PR (ms)", + "Δ", + "noise(m/p)", + "flag", + ] + table_rows = [ + [str(r.axes.get(axis, "")) for axis in axis_keys] + + [ + f"{r.main_ms:.3f}", + f"{r.pr_ms:.3f}", + pct(r.delta), + f"{r.main_noise * 100:.2f}%/{r.pr_noise * 100:.2f}%", + r.flag(threshold), + ] + for r in sorted( + rows, + key=lambda r: tuple(str(r.axes.get(a, "")) for a in axis_keys), + ) + ] + out.append(f"### `{bench}`\n\n") + out.append(markdown_table(headers, table_rows)) + out.append("\n") + return "".join(out) + + +def render_report( + suites: list[SuiteResult], threshold: float, top: int +) -> str: + parts = [ + render_header(threshold), + render_summary(suites, threshold), + render_top(suites, top), + ] + parts += [render_suite(suite, threshold) for suite in suites] + return "".join(parts) + + +def print_significant(suites: list[SuiteResult], threshold: float) -> None: + print("\n=== Significant differences ===") + found = False + for suite in suites: + for r in suite.rows: + if r.significant(threshold): + found = True + tag = "FASTER" if r.delta < 0 else "SLOWER" + print( + f" [{tag}] {suite.name}/{r.bench} {axes_str(r.axes)} " + f"main={r.main_ms:.3f}ms PR={r.pr_ms:.3f}ms d={pct(r.delta)}" + ) + if not found: + print(" (none — all within noise / below threshold)") + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser( + description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + parser.add_argument( + "--pr", required=True, type=Path, help="dir with PR-branch CSVs" + ) + parser.add_argument( + "--main", required=True, type=Path, help="dir with main-branch CSVs" + ) + parser.add_argument( + "--report", required=True, type=Path, help="output markdown path" + ) + parser.add_argument( + "--threshold", + type=float, + default=0.05, + help="significance threshold (default 0.05)", + ) + parser.add_argument( + "--top", type=int, default=15, help="rows in Top-N table (default 15)" + ) + return parser.parse_args() + + +def main() -> None: + args = parse_args() + + suite_names = sorted(p.stem for p in args.pr.glob("*.csv")) + if not suite_names: + raise SystemExit(f"no CSVs found in {args.pr}") + + suites = [ + s + for name in suite_names + if (s := compare_suite(name, args.pr, args.main)) is not None + ] + + args.report.parent.mkdir(parents=True, exist_ok=True) + args.report.write_text(render_report(suites, args.threshold, args.top)) + print(f"wrote {args.report}") + + print_significant(suites, args.threshold) + + +if __name__ == "__main__": + main() From 0bd23d187077a225e773199f9198d737dc5c47cd Mon Sep 17 00:00:00 2001 From: Muhammad Haseeb <14217455+mhaseeb123@users.noreply.github.com> Date: Tue, 2 Jun 2026 00:56:29 +0000 Subject: [PATCH 2/6] Style fix --- .agents/skills/perf-compare-cudf/scripts/compare.py | 5 +++-- pyproject.toml | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.agents/skills/perf-compare-cudf/scripts/compare.py b/.agents/skills/perf-compare-cudf/scripts/compare.py index ed1b46fbaafa..f27779539df1 100644 --- a/.agents/skills/perf-compare-cudf/scripts/compare.py +++ b/.agents/skills/perf-compare-cudf/scripts/compare.py @@ -127,7 +127,7 @@ def config_key(row: dict): axes = sorted( c for c in row if c not in NON_AXIS and not looks_like_metric(c) ) - return (row[COL_BENCHMARK],) + tuple((axis, row[axis]) for axis in axes) + return (row[COL_BENCHMARK], *((axis, row[axis]) for axis in axes)) def compare_suite( @@ -247,7 +247,8 @@ def render_suite(suite: SuiteResult, threshold: float) -> str: for bench, rows in by_bench.items(): axis_keys = sorted({axis for r in rows for axis in r.axes}) - headers = axis_keys + [ + headers = [ + *axis_keys, "main (ms)", "PR (ms)", "Δ", diff --git a/pyproject.toml b/pyproject.toml index 930f81b62937..d8d5c0bb7020 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -162,3 +162,4 @@ extend-unsafe-fixes = [ "ci/*" = ["T201"] "docs/*" = ["T201"] "cpp/*" = ["T201"] +".agents/*" = ["T201"] From f23f3c362c0f77052eafae45f3f517f15a62a6da Mon Sep 17 00:00:00 2001 From: Muhammad Haseeb Date: Thu, 25 Jun 2026 00:05:31 +0000 Subject: [PATCH 3/6] Address review comments --- .agents/skills/perf-compare-cudf/SKILL.md | 273 +++++--------- .../perf-compare-cudf/scripts/compare.py | 353 ------------------ 2 files changed, 90 insertions(+), 536 deletions(-) delete mode 100644 .agents/skills/perf-compare-cudf/scripts/compare.py diff --git a/.agents/skills/perf-compare-cudf/SKILL.md b/.agents/skills/perf-compare-cudf/SKILL.md index e182601cec61..a4321b10c5b9 100644 --- a/.agents/skills/perf-compare-cudf/SKILL.md +++ b/.agents/skills/perf-compare-cudf/SKILL.md @@ -1,215 +1,122 @@ --- name: perf-compare-cudf -description: Use when the user invokes /perf-compare-cudf, asks to benchmark a cudf branch or PR against main, or wants a performance comparison for libcudf changes. +description: Benchmark a cuDF branch, WIP changes, or a PR against the `main` branch --- -Use this skill when the user invokes `/perf-compare-cudf` with either: -- **nothing / "current branch"** (default) — benchmark the currently checked-out branch, or -- **a cudf PR link / number** (e.g. `https://github.com/rapidsai/cudf/pull/12345`) — check out that PR first. +Use this skill when the user asks to compare libcudf benchmark performance for: +- **the current branch or WIP changes** against `rapidsai/cudf` `main`. +- **a cudf PR link or number** against `rapidsai/cudf` `main`. # Goal -Run a curated subset of nvbench benchmarks on the PR branch, then on a branch in sync with `/main`, and produce a markdown report comparing the two with significant changes highlighted. -Note the cudf GH repository link: https://github.com/rapidsai/cudf -`` = the git remote pointing at cudf GH repo (often `upstream`). Detect it with `git remote -v`. +Run the same selected libcudf NVBench benchmarks on the target and then on `rapidsai/cudf` `main`, then report meaningful differences. ---- - -## Workflow checklist - -Copy and track progress: - -``` -- [ ] 0. Confirm devcontainer + env (/build-test-cudf skill) -- [ ] 1. Determine target + which case applies (A: target is current branch / B: target is a different PR branch) -- [ ] 2. Get onto the PR side per the case (Case A: stay put, keep local changes; Case B: record start branch, stash, checkout clean PR branch) -- [ ] 3. Decide which benchmarks to run (infer from diff; confirm with user) -- [ ] 4. Decide axis coverage (full vs skim); confirm with user -- [ ] 5. Pick an idle GPU (nvidia-smi) -- [ ] 6. Build libcudf with -DBUILD_BENCHMARKS=ON on PR side -- [ ] 7. Run benchmarks on PR side -> results/pr/ -- [ ] 8. Stash now (Case A only — preserve local changes), then switch to a branch in sync with latest /main -- [ ] 9. Rebuild libcudf with benchmarks on main -- [ ] 10. Run the SAME benchmark invocations on main -> results/main/ -- [ ] 11. Generate comparison report -- [ ] 12. Restore: return to the starting branch, unstash; clean up any temp branch created -``` - ---- - -## Step 0: Devcontainer + build environment - -Read and follow `/build-test-cudf` (skill at `.agents/skills/build-test-cudf/SKILL.md`) to: -- Confirm we are in a cudf devcontainer (username `coder`). If not, stop. -- Get and follow instructions to build (and troubleshoot builds) libcudf with specified CMake options - -## Step 1: Determine target and which case applies - -Record the starting branch (`git branch --show-current`). Then classify into one of two cases — this decides how local changes and stashing are handled: - -- **Case A — target is the current branch.** Applies when no PR was given, OR a PR was given and the current branch already is the PR's branch (tracks it). **Keep local uncommitted changes as-is; do NOT stash yet.** We build & benchmark the PR side first, then stash only when leaving for main. -- **Case B — target is a PR on a different branch.** Applies when a PR was given and the current branch is NOT the PR's branch. - -For a PR, resolve its head branch (to compare against current and to check out in Case B). Use `gh` if available: - -```bash -gh pr view --repo rapidsai/cudf --json headRefName,headRepository,headRepositoryOwner,baseRefName,title -``` - -If `gh` auth is unavailable, fetch the PR ref directly: - -```bash -git fetch pull//head:pr- -``` - -## Step 2: Get onto the PR side - -- **Case A:** stay on the current branch. Do not stash and do not checkout — local changes stay applied through the PR-side build and benchmark run. -- **Case B:** the PR side must be clean, so: - 1. Record the starting branch and stash any uncommitted changes, **recording exactly what was stashed** so it can be restored at the very end: - - ```bash - git stash push -m "perf-compare-cudf: wip" -- # or `git stash push -m ...` for everything - git stash list - ``` - 2. Check out the PR branch: reuse an existing local branch tracking it; otherwise check out the fetched ref (`git checkout pr-`). `git checkout ` works only when a local/remote-tracking branch of that name already exists (e.g. same-repo PRs). - -## Step 3: Decide which benchmarks to run - -Infer candidate benchmarks from the diff vs `/main`: - -```bash -git fetch main -git diff --name-only /main...HEAD -``` - -Map changed source areas to benchmark binaries. Benchmarks are built at `cpp/build/latest/benchmarks`. List all with `ls cpp/build/latest/benchmarks` and inspect a binary's benchmarks/axes with ` --list`. - -Examples of mapping: -- Parquet writer changes → `PARQUET_WRITER_NVBENCH` -- Parquet reader / cuIO source changes → `PARQUET_READER_NVBENCH`, `PARQUET_READER_COMPRESSED_NVBENCH`, `PARQUET_MULTITHREAD_READER_NVBENCH`, `HYBRID_SCAN_*_NVBENCH`, etc. -- Join changes → `JOIN_NVBENCH`; sort → `SORT_NVBENCH`; strings → `STRINGS_NVBENCH`. - -**Always confirm the benchmark list with the user** (use AskQuestion if available). The user may name specific benchmarks, give a hint, or ask to run all of them. +`` is the git remote for `https://github.com/rapidsai/cudf` (often `upstream`). Detect it with `git remote -v`. -## Step 4: Decide axis coverage - -Each nvbench benchmark has axes with many values; full cross-product is large. Ask the user (AskQuestion) whether to: -- **Skim** (default, fast): pick a few representative values per axis — smallest, largest, and a couple in the middle. -- **Full**: run all configurations. - -Inspect axes per binary: - -```bash -cpp/build/latest/benchmarks/ --list # shows axes + values -cpp/build/latest/benchmarks/ --help-axes # axis spec syntax -``` - -Override axes with `-a name=[v1,v2,...]` (and `-b ` to scope). **Record the exact axis values chosen** and use the identical invocation on both branches. - -## Step 5: Pick an idle GPU - -```bash -nvidia-smi --query-gpu=index,name,utilization.gpu,memory.used,memory.total --format=csv -``` - -Choose one idle GPU (0% util, ~0 MiB used). Run on a single device only. Pass `CUDA_VISIBLE_DEVICES=` and also `-d 0` to every nvbench invocation (after masking, the chosen device is index 0). - -## Step 6: Build libcudf (PR side) - -Use the `/build-test-cudf` skill to configure libcudf with `-DBUILD_BENCHMARKS=ON` CMake option and build. **Do not miss** this CMake option as libcudf benchmarks will not build otherwise. In Case A this builds with the local uncommitted changes applied (intended). In case of errors, use the same skill to troubleshoot or clean, reconfigure **with the CMake option** and rebuild as needed. - -## Step 7: Run benchmarks on PR side - -Write CSV + log per benchmark binary into a results dir (suggest `.agents/benchmark-results//pr/`). For each benchmark binary: - -```bash -CUDA_VISIBLE_DEVICES= cpp/build/latest/benchmarks/ -d 0 \ - -b -a ... \ - --csv /pr/.csv 2>&1 | tee /pr/.log -``` - -Notes: -- nvbench may emit a benign segfault at the very end of a suite — **ignore end-of-suite segfaults**. -- If a config throws an exception, note it and exclude it from the comparison, but verify the **same** behavior occurs on both branches. - -## Step 8: Stash (Case A) and switch to a branch in sync with rapidsai/cudf main - -Now that PR-side data is collected, leave for the main side. The "main" side must reflect `/main` (rapidsai/cudf), not a stale fork main, and must NOT carry the PR's changes. - -1. **Case A only:** the local changes are still applied — stash them now so main is clean, **recording exactly what was stashed**: +## Prerequisites +- **`gh` CLI** authenticated — run `gh auth status`. If not authenticated, guide the user to run: ```bash - git stash push -m "perf-compare-cudf: wip" -- # or `git stash push -m ...` for everything - git stash list + gh auth login ``` + The token needs `repo` scope. Do **not** run `gh auth token` from within the agent. +- Ensure we are in cudf devcontainer. Otherwise ensure that the CUDA, compilers, and cudf build helpers are available. - (Case B already stashed in Step 2 and is on a clean PR branch.) -2. `git fetch main` -3. Decide the main-side ref: - - If local `main` already tracks `/main` and is up to date → `git checkout main`. - - If local `main` tracks a fork → pull `/main` into it, **or** create a temp branch from the remote: `git checkout -b _bench_main /main` (delete it in Step 12). -4. Do NOT apply the stash on main — main must stay clean of PR changes so the comparison is meaningful. +## 1. Prepare -## Step 9: Rebuild on main +- Record the starting branch, `git status --short`, and the exact target (current WIP or cudf PR). +- Before switching branches, stash unrelated user changes and record the stash name. If the target is the current WIP, keep changes applied for the target run, then stash them before switching to main. +- Check out the PR with: + ```bash + gh pr checkout --repo rapidsai/cudf + ``` -Follow the instructions in **Step 6: Build libcudf (PR side)** to properly configure and rebuild on the "main" branch. Note that the "main" branch must be in a clean state. +## 2. Choose Benchmarks -## Step 10: Run the SAME benchmarks on main +- Fetch current main with `git fetch main`. +- Infer candidates from `git diff --name-only /main...HEAD`. +- Build outputs live under `cpp/build/latest/benchmarks/*_NVBENCH`. +- Inspect a binary with: + ```bash + cpp/build/latest/benchmarks/ --list + cpp/build/latest/benchmarks/ --help-axes + ``` +- Confirm benchmark binaries and axis coverage with the user. Use a small, representative axis subset by default; use full coverage only when requested or necessary. +- Record exact `-b` and `-a` options. Reuse them unchanged on both branches. -Use the identical `-b`/`-a` invocations and the same GPU, writing to `/main/`. +## 3. Build each side -## Step 11: Generate the comparison report - -Use the helper script (reads matching CSVs and emits markdown): +On both target and main, force CMake reconfiguration to enable benchmarks via `-DBUILD_BENCHMARKS=ON` before building: ```bash -python .agents/skills/perf-compare-cudf/scripts/compare.py \ - --pr /pr --main /main --report /COMPARISON.md +configure-cudf-cpp -DBUILD_BENCHMARKS=ON +build-cudf-cpp ``` -The script matches rows by `(benchmark, axis-values)`, computes `Δ = (PR - main) / main` on GPU time, and flags **significant** changes where `|Δ| >= 5%` AND `|Δ| >` max(noise of either side). It prints significant rows to stdout. - -**Re-run any flagged config once** to confirm it is not noise (especially sub-millisecond benches with high nvbench noise). If the rerun matches within noise, replace that CSV row's run and regenerate. +Use `/build-test-cudf` skill for configure, build instructions and troubleshooting. + +## 4. Run each side + +- Pick an idle GPU with `nvidia-smi`. Re-check every time before running anything (target or main run); if the same GPU is no longer idle, pick another one, wait or ask before continuing. +- Run one masked device only: `CUDA_VISIBLE_DEVICES=` and `-d 0`. +- Write matching JSON and log files, for example: + ```bash + CUDA_VISIBLE_DEVICES= cpp/build/latest/benchmarks/ -d 0 \ + -b -a ... \ + --json /pr/.json 2>&1 | tee /pr/.log + ``` +- To switch to main, stash target WIP if needed, then use a clean branch: + ```bash + git fetch main + git checkout -B _bench_main /main + ``` + Do not apply target changes on `_bench_main`. +- Repeat the identical command on main, writing to `/main/`. +- If nvbench emits an end-of-suite segfault after writing results, note it and continue. If a config throws, verify whether both branches behave the same. + +## 5. Compare +Use NVBench's comparison script from the build tree: -See the report template at the bottom. - -## Step 12: Restore state - -- Return to the starting branch recorded in Step 1 (Case A: the original/current branch; Case B: the branch we started on before checking out the PR). -- Pop the stash to restore the user's uncommitted changes (both cases stashed something: Case A in Step 8, Case B in Step 2). -- Delete any temp branch created in Step 8 (e.g. `git branch -D _bench_main`). -- Confirm `git status` matches the pre-run state. - ---- +```bash +NVBENCH_SCRIPTS=cpp/build/latest/_deps/nvbench-src/python/scripts +test -f "$NVBENCH_SCRIPTS/nvbench_compare.py" || \ + NVBENCH_SCRIPTS=cpp/build/latest/_deps/nvbench-src/scripts +PYTHONPATH="$NVBENCH_SCRIPTS" python "$NVBENCH_SCRIPTS/nvbench_compare.py" \ + --threshold-diff 0.05 --no-color /main /pr \ + | tee /COMPARISON.md +``` -## Report template +The first path is the reference (`main`), the second is the comparison (`pr`). Re-run surprising failures once, especially small or noisy configs. -The generated `COMPARISON.md` (and your chat summary) should include: +## 6. Restore and report -```markdown -# Benchmark Comparison: /main vs PR (``) +- Return to the starting branch, pop any stash you created, delete temporary branches, and confirm `git status` matches the starting state. +- Summarize chat with the headline result (regression, improvement, or within noise), relevant metrics, hardware used, branch SHAs, axis coverage, and generated files. +- Use this report shape for `COMPARISON.md`, adapting the metric columns to the benchmark. GPU time is always useful, but other metrics such as output file size, throughput, compression ratio, or memory usage are also of interest when they change significantly in target vs main. -- GPU Time in ms. Δ = (PR - main) / main. Negative = PR faster. -- Significant: |Δ| >= 5% AND larger than max(noise) of either side. -- Hardware: , driver/CUDA if available. -- Branches: PR `` vs main ``. Axis coverage: (list values used). + ```markdown + # Benchmark Comparison: /main vs target (`WIP` or `PR`) -## Summary -| Benchmark Suite | # Configs | # Significant | -| ... | + - Primary metric(s): + - Δ = (target - main) / main. Interpret direction per metric. + - Significant timing deltas: |Δ| >= 5% AND larger than max(noise) of either side. + - Hardware: , driver/CUDA if available. + , model, architecture, if available. + - Branches: target `` vs main ``. Axis coverage: (list values used). -## Top N by |Δ| -| Suite / bench | axes | main (ms) | PR (ms) | Δ | noise(m/p) | -| ... | + ## Summary + | Benchmark Suite | Primary metric | # Configs | # Meaningful Changes | + | ... | -## Per-suite tables -（one table per benchmark, axes as columns, with FASTER/SLOWER flags） + ## Top N Changes + | Suite / bench | axes | metric | main | target | Δ | noise, if timing | + | ... | -## Notes -- Exceptions excluded (same on both branches): ... -- End-of-suite segfaults ignored. -- Files generated: list of CSV/log paths + this report + compare.py -``` + ## Per-suite tables + (one table per benchmark, axes as columns, include all relevant metrics) -Always end the chat summary with: the headline (regression / improvement / within-noise), the hardware, the axis coverage, and the list of generated files. + ## Notes + - Exceptions excluded (same on both branches): ... + - End-of-suite segfaults ignored. + - Files generated: list of JSON/log paths + this report + ``` diff --git a/.agents/skills/perf-compare-cudf/scripts/compare.py b/.agents/skills/perf-compare-cudf/scripts/compare.py deleted file mode 100644 index f27779539df1..000000000000 --- a/.agents/skills/perf-compare-cudf/scripts/compare.py +++ /dev/null @@ -1,353 +0,0 @@ -#!/usr/bin/env python3 - -# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION. -# SPDX-License-Identifier: Apache-2.0 - -"""Compare cudf nvbench CSV results between a `main` run and a `pr` run. - -Rows are matched by (benchmark name, axis values). For each matched config we -compute the GPU-time delta (PR - main) / main and emit a markdown report. A -config is flagged "significant" when |delta| >= threshold AND |delta| exceeds -the nvbench noise on either side, so noise-dominated swings don't get flagged. - -Each input directory holds one CSV per benchmark binary, named after the binary -(as produced by ` --csv /.csv`). Only CSVs present in BOTH -directories (matched by filename) are compared. -""" - -import argparse -import csv -from dataclasses import dataclass -from pathlib import Path - -# nvbench CSV column names we read directly. -COL_BENCHMARK = "Benchmark" -COL_GPU_TIME = "GPU Time (sec)" -COL_NOISE = "Noise" -COL_SKIPPED = "Skipped" - -# Columns that are never benchmark axes (identity + the metrics we don't axis on). -NON_AXIS = { - COL_BENCHMARK, - "Device", - "Device Name", - COL_SKIPPED, - "Samples", - "CPU Time (sec)", - COL_NOISE, - COL_GPU_TIME, -} - -# nvbench emits a different set of metric/output columns per benchmark, so we -# can't enumerate them. Instead we drop any column whose name looks like a -# measured output; everything else is treated as an axis. -METRIC_HINTS = ( - "per_second", - "per_sec", - "bytes_per_second", - "BW", - "Noise", - "Samples", - "Time (sec)", - "memory", - "encoded", - "Util", - "size (bytes)", - "throughput", -) - - -def looks_like_metric(column: str) -> bool: - return any(hint.lower() in column.lower() for hint in METRIC_HINTS) - - -def pct(value: float) -> str: - return f"{value * 100:+.2f}%" - - -def axes_str(axes: dict[str, str]) -> str: - return " ".join( - f"{name}={value}" for name, value in axes.items() if value != "" - ) - - -def markdown_table(headers: list[str], rows: list[list[str]]) -> str: - """Render a GitHub-flavored markdown table from headers and string cells.""" - lines = [ - "| " + " | ".join(headers) + " |", - "|" + "|".join("---" for _ in headers) + "|", - ] - lines += ["| " + " | ".join(cells) + " |" for cells in rows] - return "\n".join(lines) + "\n" - - -@dataclass -class Comparison: - """One benchmark config measured on both branches.""" - - bench: str - axes: dict[str, str] - main_ms: float - pr_ms: float - delta: float - main_noise: float - pr_noise: float - - def significant(self, threshold: float) -> bool: - return abs(self.delta) >= threshold and abs(self.delta) > max( - self.main_noise, self.pr_noise - ) - - def flag(self, threshold: float) -> str: - if not self.significant(threshold): - return "" - return "**FASTER**" if self.delta < 0 else "**SLOWER**" - - def noise_str(self) -> str: - return f"{self.main_noise * 100:.1f}%/{self.pr_noise * 100:.1f}%" - - -@dataclass -class SuiteResult: - """Comparison outcome for a single benchmark binary.""" - - name: str - rows: list[Comparison] - only_pr: list # config keys present only in the PR run - only_main: list # config keys present only in the main run - - -def read_rows(path: Path) -> list[dict]: - with open(path) as f: - return list(csv.DictReader(f)) - - -def config_key(row: dict): - """Identity of a benchmark config: its name plus all (axis, value) pairs.""" - axes = sorted( - c for c in row if c not in NON_AXIS and not looks_like_metric(c) - ) - return (row[COL_BENCHMARK], *((axis, row[axis]) for axis in axes)) - - -def compare_suite( - name: str, pr_dir: Path, main_dir: Path -) -> SuiteResult | None: - pr_csv, main_csv = pr_dir / f"{name}.csv", main_dir / f"{name}.csv" - if not pr_csv.exists() or not main_csv.exists(): - return None - - pr_by_key = {config_key(row): row for row in read_rows(pr_csv)} - main_by_key = {config_key(row): row for row in read_rows(main_csv)} - - rows: list[Comparison] = [] - for key in sorted(pr_by_key.keys() & main_by_key.keys()): - pr_row, main_row = pr_by_key[key], main_by_key[key] - if ( - pr_row.get(COL_SKIPPED) == "Yes" - or main_row.get(COL_SKIPPED) == "Yes" - ): - continue - try: - pr_time = float(pr_row[COL_GPU_TIME]) - main_time = float(main_row[COL_GPU_TIME]) - except (KeyError, ValueError): - continue - if main_time <= 0: - continue - rows.append( - Comparison( - bench=key[0], - axes={axis: value for axis, value in key[1:]}, - main_ms=main_time * 1000, - pr_ms=pr_time * 1000, - delta=(pr_time - main_time) / main_time, - main_noise=float(main_row.get(COL_NOISE) or 0), - pr_noise=float(pr_row.get(COL_NOISE) or 0), - ) - ) - - return SuiteResult( - name=name, - rows=rows, - only_pr=sorted(pr_by_key.keys() - main_by_key.keys()), - only_main=sorted(main_by_key.keys() - pr_by_key.keys()), - ) - - -def render_header(threshold: float) -> str: - return ( - "# Benchmark Comparison: main vs PR\n\n" - "- GPU Time in ms. Δ = (PR - main) / main. Negative = PR faster.\n" - f"- **Significant**: |Δ| >= {threshold * 100:.0f}% AND larger than max(noise) of either side.\n" - "- Fill in hardware (GPU/driver/CUDA), branch SHAs, and axis coverage manually.\n\n" - ) - - -def render_summary(suites: list[SuiteResult], threshold: float) -> str: - rows = [ - [ - suite.name, - str(len(suite.rows)), - str(sum(1 for r in suite.rows if r.significant(threshold))), - ] - for suite in suites - ] - return ( - "## Summary\n\n" - + markdown_table( - ["Benchmark Suite", "# Configs", "# Significant"], rows - ) - + "\n" - ) - - -def render_top(suites: list[SuiteResult], top: int) -> str: - everything = [(suite.name, row) for suite in suites for row in suite.rows] - everything.sort(key=lambda pair: abs(pair[1].delta), reverse=True) - rows = [ - [ - f"{suite_name}/{r.bench}", - axes_str(r.axes), - f"{r.main_ms:.3f}", - f"{r.pr_ms:.3f}", - pct(r.delta), - r.noise_str(), - ] - for suite_name, r in everything[:top] - ] - return ( - f"## Top {top} by |Δ|\n\n" - + markdown_table( - [ - "Suite / bench", - "axes", - "main (ms)", - "PR (ms)", - "Δ", - "noise(m/p)", - ], - rows, - ) - + "\n" - ) - - -def render_suite(suite: SuiteResult, threshold: float) -> str: - out = [f"## {suite.name}\n\n"] - if suite.only_pr or suite.only_main: - out.append( - f"_only-on-PR configs: {len(suite.only_pr)}, " - f"only-on-main: {len(suite.only_main)}_\n\n" - ) - - by_bench: dict[str, list[Comparison]] = {} - for row in suite.rows: - by_bench.setdefault(row.bench, []).append(row) - - for bench, rows in by_bench.items(): - axis_keys = sorted({axis for r in rows for axis in r.axes}) - headers = [ - *axis_keys, - "main (ms)", - "PR (ms)", - "Δ", - "noise(m/p)", - "flag", - ] - table_rows = [ - [str(r.axes.get(axis, "")) for axis in axis_keys] - + [ - f"{r.main_ms:.3f}", - f"{r.pr_ms:.3f}", - pct(r.delta), - f"{r.main_noise * 100:.2f}%/{r.pr_noise * 100:.2f}%", - r.flag(threshold), - ] - for r in sorted( - rows, - key=lambda r: tuple(str(r.axes.get(a, "")) for a in axis_keys), - ) - ] - out.append(f"### `{bench}`\n\n") - out.append(markdown_table(headers, table_rows)) - out.append("\n") - return "".join(out) - - -def render_report( - suites: list[SuiteResult], threshold: float, top: int -) -> str: - parts = [ - render_header(threshold), - render_summary(suites, threshold), - render_top(suites, top), - ] - parts += [render_suite(suite, threshold) for suite in suites] - return "".join(parts) - - -def print_significant(suites: list[SuiteResult], threshold: float) -> None: - print("\n=== Significant differences ===") - found = False - for suite in suites: - for r in suite.rows: - if r.significant(threshold): - found = True - tag = "FASTER" if r.delta < 0 else "SLOWER" - print( - f" [{tag}] {suite.name}/{r.bench} {axes_str(r.axes)} " - f"main={r.main_ms:.3f}ms PR={r.pr_ms:.3f}ms d={pct(r.delta)}" - ) - if not found: - print(" (none — all within noise / below threshold)") - - -def parse_args() -> argparse.Namespace: - parser = argparse.ArgumentParser( - description=__doc__, - formatter_class=argparse.RawDescriptionHelpFormatter, - ) - parser.add_argument( - "--pr", required=True, type=Path, help="dir with PR-branch CSVs" - ) - parser.add_argument( - "--main", required=True, type=Path, help="dir with main-branch CSVs" - ) - parser.add_argument( - "--report", required=True, type=Path, help="output markdown path" - ) - parser.add_argument( - "--threshold", - type=float, - default=0.05, - help="significance threshold (default 0.05)", - ) - parser.add_argument( - "--top", type=int, default=15, help="rows in Top-N table (default 15)" - ) - return parser.parse_args() - - -def main() -> None: - args = parse_args() - - suite_names = sorted(p.stem for p in args.pr.glob("*.csv")) - if not suite_names: - raise SystemExit(f"no CSVs found in {args.pr}") - - suites = [ - s - for name in suite_names - if (s := compare_suite(name, args.pr, args.main)) is not None - ] - - args.report.parent.mkdir(parents=True, exist_ok=True) - args.report.write_text(render_report(suites, args.threshold, args.top)) - print(f"wrote {args.report}") - - print_significant(suites, args.threshold) - - -if __name__ == "__main__": - main() From 1b9ec92b15d0dcb2dccd7704cb5b703ad86849a4 Mon Sep 17 00:00:00 2001 From: Muhammad Haseeb Date: Thu, 25 Jun 2026 00:10:18 +0000 Subject: [PATCH 4/6] Unneeded --- pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 3050a615be18..91e516ca7b84 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -167,7 +167,6 @@ extend-unsafe-fixes = [ "ci/*" = ["T201"] "docs/*" = ["T201"] "cpp/*" = ["T201"] -".agents/*" = ["T201"] [tool.cython-lint] max-line-length = 120 From 908a991a1ec315f1e50a1a073dff0de4f8128ee8 Mon Sep 17 00:00:00 2001 From: Muhammad Haseeb Date: Fri, 26 Jun 2026 19:59:03 +0000 Subject: [PATCH 5/6] Apply suggested improvements --- .agents/skills/perf-compare-cudf/SKILL.md | 89 ++++++++++++++--------- 1 file changed, 53 insertions(+), 36 deletions(-) diff --git a/.agents/skills/perf-compare-cudf/SKILL.md b/.agents/skills/perf-compare-cudf/SKILL.md index a4321b10c5b9..1d2ab3f96147 100644 --- a/.agents/skills/perf-compare-cudf/SKILL.md +++ b/.agents/skills/perf-compare-cudf/SKILL.md @@ -9,72 +9,88 @@ Use this skill when the user asks to compare libcudf benchmark performance for: # Goal -Run the same selected libcudf NVBench benchmarks on the target and then on `rapidsai/cudf` `main`, then report meaningful differences. +Run the same selected libcudf NVBench benchmarks on the target (current WIP or cudf PR) and then on `rapidsai/cudf` `main`, then report meaningful differences. `` is the git remote for `https://github.com/rapidsai/cudf` (often `upstream`). Detect it with `git remote -v`. ## Prerequisites -- **`gh` CLI** authenticated — run `gh auth status`. If not authenticated, guide the user to run: +- For PR targets, **`gh` CLI** authenticated — run `gh auth status`. If not authenticated, guide the user to run: ```bash gh auth login ``` The token needs `repo` scope. Do **not** run `gh auth token` from within the agent. -- Ensure we are in cudf devcontainer. Otherwise ensure that the CUDA, compilers, and cudf build helpers are available. +- Ensure we are in the cudf devcontainer (username `coder`). If not, stop and ask the user for instructions. ## 1. Prepare - Record the starting branch, `git status --short`, and the exact target (current WIP or cudf PR). -- Before switching branches, stash unrelated user changes and record the stash name. If the target is the current WIP, keep changes applied for the target run, then stash them before switching to main. -- Check out the PR with: +- Run order: Target side first, then `main`. +- Record current timestamp as `ts = ` +- Create result directories: ```bash - gh pr checkout --repo rapidsai/cudf + mkdir -p benchmark_compare//{target,main} ``` -## 2. Choose Benchmarks +## 2. Build Target -- Fetch current main with `git fetch main`. -- Infer candidates from `git diff --name-only /main...HEAD`. -- Build outputs live under `cpp/build/latest/benchmarks/*_NVBENCH`. -- Inspect a binary with: +- For current-branch or WIP targets: keep target changes applied for the target run. +- For PR targets: Stash any unrelated local changes, record the stash name, and check out the PR: ```bash - cpp/build/latest/benchmarks/ --list - cpp/build/latest/benchmarks/ --help-axes + gh pr checkout --repo rapidsai/cudf ``` -- Confirm benchmark binaries and axis coverage with the user. Use a small, representative axis subset by default; use full coverage only when requested or necessary. -- Record exact `-b` and `-a` options. Reuse them unchanged on both branches. - -## 3. Build each side - -On both target and main, force CMake reconfiguration to enable benchmarks via `-DBUILD_BENCHMARKS=ON` before building: +- On the first build for a checkout, force CMake reconfiguration to enable benchmarks: ```bash configure-cudf-cpp -DBUILD_BENCHMARKS=ON build-cudf-cpp ``` +- Re-run `configure-cudf-cpp -DBUILD_BENCHMARKS=ON` if the build directory is cleaned or CMake options may have changed. +- If needed, refer to the `/build-test-cudf` skill for more instructions and troubleshooting. -Use `/build-test-cudf` skill for configure, build instructions and troubleshooting. +## 3. Choose Benchmarks + +- Fetch current main with `git fetch main`. +- Infer candidate benchmark suites from: + ```bash + git diff --name-only /main...HEAD + ``` +- Benchmark binaries live under `cpp/build/latest/benchmarks/*_NVBENCH`. +- Inspect candidate binaries from the target build: + ```bash + cpp/build/latest/benchmarks/ --list + cpp/build/latest/benchmarks/ --help-axes + ``` +- Confirm benchmark binaries and axis coverage with the user. Use a small, representative axis subset by default; use full coverage only when requested or necessary. +- Record exact `-b` and `-a` options. Reuse them unchanged on both branches. -## 4. Run each side +## 4. Run Target -- Pick an idle GPU with `nvidia-smi`. Re-check every time before running anything (target or main run); if the same GPU is no longer idle, pick another one, wait or ask before continuing. -- Run one masked device only: `CUDA_VISIBLE_DEVICES=` and `-d 0`. -- Write matching JSON and log files, for example: +- Pick an idle GPU with `nvidia-smi`. Do this every time before running anything (target or main run); if the same GPU is no longer idle, pick another one, wait, or ask before continuing. +- Run on one masked device only: `CUDA_VISIBLE_DEVICES=` and `-d 0`. +- Write target JSON and log files under `benchmark_compare//target/`, for example: ```bash CUDA_VISIBLE_DEVICES= cpp/build/latest/benchmarks/ -d 0 \ -b -a ... \ - --json /pr/.json 2>&1 | tee /pr/.log + --json benchmark_compare//target/.json 2>&1 | tee benchmark_compare//target/.log ``` -- To switch to main, stash target WIP if needed, then use a clean branch: +- If nvbench emits an end-of-suite segfault after writing results, note it and continue. If a config throws, verify that both branches (main and target) behave the same. + +## 5. Switch over to main + +- To switch to main, stash any target WIP if needed, record the stash name, and use a clean branch: ```bash git fetch main git checkout -B _bench_main /main ``` - Do not apply target changes on `_bench_main`. -- Repeat the identical command on main, writing to `/main/`. -- If nvbench emits an end-of-suite segfault after writing results, note it and continue. If a config throws, verify whether both branches behave the same. +- Do not apply any WIP or target changes on `_bench_main`. + +## 6. Build and run main + +Follow configure, build and benchmark run steps as for the target. Run the same set of benchmarks chosen above, but write JSON and log files to `benchmark_compare//main/` instead. + +## 7. Compare -## 5. Compare Use NVBench's comparison script from the build tree: ```bash @@ -82,17 +98,18 @@ NVBENCH_SCRIPTS=cpp/build/latest/_deps/nvbench-src/python/scripts test -f "$NVBENCH_SCRIPTS/nvbench_compare.py" || \ NVBENCH_SCRIPTS=cpp/build/latest/_deps/nvbench-src/scripts PYTHONPATH="$NVBENCH_SCRIPTS" python "$NVBENCH_SCRIPTS/nvbench_compare.py" \ - --threshold-diff 0.05 --no-color /main /pr \ - | tee /COMPARISON.md + --threshold-diff 0.05 --no-color benchmark_compare//main benchmark_compare//target \ + | tee benchmark_compare//COMPARISON.md ``` -The first path is the reference (`main`), the second is the comparison (`pr`). Re-run surprising failures once, especially small or noisy configs. +- The first path is the reference (`main`), the second is the comparison (`target`). Re-run surprising failures once, especially small or noisy configs. -## 6. Restore and report +## 8. Restore and report -- Return to the starting branch, pop any stash you created, delete temporary branches, and confirm `git status` matches the starting state. +- Return to the starting branch/state, pop any stash you created, delete temporary branches, and confirm `git status` matches the starting state. - Summarize chat with the headline result (regression, improvement, or within noise), relevant metrics, hardware used, branch SHAs, axis coverage, and generated files. -- Use this report shape for `COMPARISON.md`, adapting the metric columns to the benchmark. GPU time is always useful, but other metrics such as output file size, throughput, compression ratio, or memory usage are also of interest when they change significantly in target vs main. +- Remember to note if there were any end-of-suite segfaults or config throws and if the behavior was the same on both branches. +- Use this template for `COMPARISON.md`, adapting the metric columns to the benchmark. GPU time is always useful, but other metrics such as output file size, throughput, compression ratio, or memory usage are also of interest when they change significantly in target vs main. ```markdown # Benchmark Comparison: /main vs target (`WIP` or `PR`) From 1fd67f6b71e8660ef6556fd33f5924b54ddfd3d5 Mon Sep 17 00:00:00 2001 From: Muhammad Haseeb Date: Fri, 26 Jun 2026 21:04:58 +0000 Subject: [PATCH 6/6] Minor polishing --- .agents/skills/perf-compare-cudf/SKILL.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.agents/skills/perf-compare-cudf/SKILL.md b/.agents/skills/perf-compare-cudf/SKILL.md index 1d2ab3f96147..30bdd2423486 100644 --- a/.agents/skills/perf-compare-cudf/SKILL.md +++ b/.agents/skills/perf-compare-cudf/SKILL.md @@ -39,6 +39,8 @@ Run the same selected libcudf NVBench benchmarks on the target (current WIP or c ```bash gh pr checkout --repo rapidsai/cudf ``` +- For PR targets: After switching, check if the PR branch is behind `/main` and add a merge commit. DO **NOT** push anything. If there are merge conflicts, stop and guide the user to fix them. + - On the first build for a checkout, force CMake reconfiguration to enable benchmarks: ```bash @@ -46,7 +48,7 @@ configure-cudf-cpp -DBUILD_BENCHMARKS=ON build-cudf-cpp ``` - Re-run `configure-cudf-cpp -DBUILD_BENCHMARKS=ON` if the build directory is cleaned or CMake options may have changed. -- If needed, refer to the `/build-test-cudf` skill for more instructions and troubleshooting. +- If needed, refer to the `build-test-cudf` skill for instructions and troubleshooting. ## 3. Choose Benchmarks @@ -107,9 +109,9 @@ PYTHONPATH="$NVBENCH_SCRIPTS" python "$NVBENCH_SCRIPTS/nvbench_compare.py" \ ## 8. Restore and report - Return to the starting branch/state, pop any stash you created, delete temporary branches, and confirm `git status` matches the starting state. -- Summarize chat with the headline result (regression, improvement, or within noise), relevant metrics, hardware used, branch SHAs, axis coverage, and generated files. - Remember to note if there were any end-of-suite segfaults or config throws and if the behavior was the same on both branches. -- Use this template for `COMPARISON.md`, adapting the metric columns to the benchmark. GPU time is always useful, but other metrics such as output file size, throughput, compression ratio, or memory usage are also of interest when they change significantly in target vs main. +- Use the below template for `COMPARISON.md`, adapting the metric columns to the benchmark. GPU time is always useful, but other metrics such as output file size, throughput, compression ratio, or memory usage are also of interest when they change significantly in target vs main. +- Summarize chat with the headline result (regression, improvement, or within noise), relevant metrics, hardware used, branch SHAs, axis coverage, the summary table from the template, and generated files. ```markdown # Benchmark Comparison: /main vs target (`WIP` or `PR`)