diff --git a/RCCL_REGRESSION_DETECTOR_UPDATE.md b/RCCL_REGRESSION_DETECTOR_UPDATE.md new file mode 100644 index 000000000..9da02659c --- /dev/null +++ b/RCCL_REGRESSION_DETECTOR_UPDATE.md @@ -0,0 +1,503 @@ +# CVS RCCL Regression Strategy (AIMVT-196) + +This document describes how the CVS RCCL performance-regression pipeline detects +real RCCL performance regressions in CI **without** false positives, including the +design rationale, the algorithm, configuration, how to run it, and the evidence +that it is trustworthy. + +- **Code (cvs)** — branch `aimvt-196-rccl-regression-robustness` (origin: `ROCm/cvs`) +- **Orchestration (cvs-sbatch)** — branch `aimvt-196-rccl-regression-robustness` (origin: `speriaswamy-amd/cvs-sbatch`) +- **Companion docs**: `RCCL_REGRESSION_FINDINGS.md` (a concrete candidate regression + bisection handoff) + +--- + +## 1. Goal & guiding principles + +Run in CI as a gate on RCCL changes and answer one question reliably: +**"Did this RCCL build get slower than a known-good reference?"** across message +sizes **1 KiB → 4 GiB**. + +Priorities, in order: + +1. **No false positives.** A flaky CI gate is worse than no gate — it erodes trust + and gets ignored/disabled. Stability is the #1 requirement. +2. **Trustworthy detection of real regressions**, especially at large messages. +3. It is **acceptable to miss small regressions** (~1–2%), particularly for small, + latency-bound messages with high run-to-run variance. + +Everything below follows from these priorities. + +--- + +## 2. Why not a static baseline? + +The previous approach compared measured bus bandwidth against **hand-maintained +expected numbers** (e.g. `330`, `350` GB/s) in config. Problems: + +- CVS has **no way to compute a baseline**, so the numbers were guesses and went stale. +- Small/mid messages are **latency-bound** and noisy; a fixed threshold either + fires on noise (false positives) or is so loose it hides real regressions. +- The comparison code also had a **group-by bug** (see §6) that silently dropped + half the data. + +**Decision: replace static baselines with paired A/B testing.** + +--- + +## 3. Core idea — paired A/B testing + +Run the **candidate** build (B) and a **reference** build (A) **back-to-back, +interleaved, on the same nodes within the same SLURM allocation**, repeated N times: + +``` +repeat 1: A B +repeat 2: A B +... +repeat N: A B +``` + +Both builds are identical except for `librccl.so` (same HIP, same MPI, same GPUs, +same fabric — selected automatically via each binary's rpath). Because A and B run +in the same time window on the same hardware, environmental noise (thermals, +neighbor jobs, NIC/fabric state, slow drift) is **common-mode and cancels in the +A−B comparison**. We never ask "is this absolute number good?" (unanswerable for +small messages); we ask "is B worse than A, side-by-side, right now?" + +This is the key to small-message stability: the *absolute* small-message bandwidth +is unstable, but the *paired difference* is not. + +--- + +## 4. The detection algorithm (triple gate) + +Implemented in `cvs/cvs/lib/regression_lib.py` (pure, dependency-free, unit-tested). + +For every fully-qualified key **`(collective, size, type, inPlace)`**, we collect a +sample of bus-bandwidth measurements for A and for B (one per repeat). A key is +flagged as a regression **only if all three independent gates agree** — the +conjunction is what makes false positives extremely unlikely: + +### Gate 1 — size-tiered relative threshold +`median(B)` must be lower than `median(A)` by more than the tier's threshold: + +| tier | size range | why | +|-------|-----------------|----------------------------------| +| small | ≤ 1 MiB | latency-bound, noisiest → loosest | +| mid | 1 MiB – 64 MiB | transitional | +| large | > 64 MiB | bandwidth-bound, stable → tightest | + +Thresholds are **derived from measured noise** (see §5), not guessed. + +### Gate 2 — non-parametric separation +Require **`p75(B) < p25(A)`** — B's upper quartile below A's lower quartile, i.e. +the two distributions barely overlap. This is a distribution-free significance test +that is robust to a single straggler run and is the specific antidote to wide, +noisy small-message distributions (which overlap and therefore won't pass). + +### Gate 3 — adjacency confirmation +A candidate size is confirmed only if it belongs to a run of **≥ `adjacency_min_run` +(default 2) consecutive candidate sizes** within the same `(collective, type, +inPlace)` group. Real regressions occupy a contiguous band of sizes; isolated noise +spikes do not. + +### Safety rails +- **Median** (not mean) over repeats → robust to outlier runs. +- **`min_bandwidth_floor`, now per tier** (`{small: 0.005, mid: 0.05, large: 0.5}` + GB/s): sizes whose busBw sits under the floor for their tier are marked + **`inconclusive`** and excluded from pass/fail — we refuse to judge the region + where no judgment is safe. A single scalar `0.5` is still accepted for old + configs, but it was a mistake: 0.5 GB/s is a *large-message* floor, and applying + it flat silently excluded the entire small tier, so the gate was scoring the + small band against thresholds calibrated on no small-band data at all. +- **`min_repeats`**: too few samples → `inconclusive`, never a regression. +- **`require_balanced_samples`**: A and B must have the same number of surviving + repeats for a key, or it is `inconclusive`. Unequal counts mean the two sides + were not measured under the same conditions, so the comparison is not paired. +- **`max_inconclusive_frac`** (0.1): if more than this fraction of a group's keys + came back inconclusive, the *group* is untrustworthy. A detector that abstained + on most of what it looked at has not established anything, and must not be + allowed to report "0 regressions" as though it had. +- Direction-aware: only flags **B worse than A**, never improvements. + +### Output +Per-key verdicts (`pass` / `regression` / `inconclusive`) with A/B medians, drop%, +the threshold used, and the reasons each gate passed/failed. Aggregated to a single +job verdict; any confirmed regression → the test fails (non-zero exit) → CI fails. + +--- + +## 5. Threshold calibration (control run) + +Thresholds are **measured on the actual hardware**, not picked by hand. + +1. Run in **control mode** (`control_mode: true`): the *reference* build is used as + **both** A and B. +2. Since A and B are the same build, any spread is pure run-to-run noise. We compute + the per-key coefficient of variation and set: + + ``` + threshold[tier] = safety_factor × (median(CV[tier]) + mad_k × MAD(CV[tier])) + threshold[tier] = min(threshold[tier], max_thresholds[tier]) # policy ceiling + ``` + + This used to be `safety_factor × p95(CV[tier])`. p95 of a CV distribution *is* + an outlier statistic — one flaky key in a tier dragged the whole tier's + threshold up and blinded the gate for every other key in it. `median + k·MAD` + (MAD scaled by 1.4826) is the robust equivalent: it tracks the bulk of the + distribution and a few bad keys cannot move it far. + + Thresholds are derived **per collective**, not globally, so the noisiest + collective no longer sets the bar for the quietest. + +3. `max_thresholds` is a hand-set ceiling. Calibration can only ever make the gate + *tighter* than this; a pathologically noisy control run cannot loosen the gate + into uselessness. It is the one threshold knob that is meant to be edited by + hand — see `ci/rccl_perf_gate/configs/README.md`. + +4. The control run **must report 0 regressions** (A vs A) **and** must be + trustworthy. A control run that tripped the circuit breaker, came back mostly + inconclusive, or failed to score every group does **not** publish its + calibration: it writes only into its own run artifacts, and logs loudly that it + withheld. Publishing from a run that failed its own checks would poison every + later detect run with a threshold nobody chose, and because that run also + reports NO VERDICT, nobody would think to go look at what it had published. + +Re-run control calibration whenever the **hardware, RCCL build, or cluster config** +changes. Calibrated values are written to `ab_derived_thresholds.json`. + +> Measured on 4-node MI355X (full matrix, job 16131): derived +> **small 12.5% / mid 6.1% / large 3.0%**, all under the `max_thresholds` ceiling +> of 15% / 8% / 6% — so the ceiling was not binding and the measured noise is what +> the gate actually uses. + +--- + +## 6. Correctness: group-by keys + +The original comparison/report code bucketed results by **message size alone**, +silently collapsing the `(data type, inPlace)` dimensions — the last row written for +a size overwrote the others. This both **hid real regressions** (overwritten rows +vanished) and **manufactured fake ones** (a data-type boundary looked like a giant +bandwidth dip). + +Fix (in `rccl_lib.py`): +- `group_rccl_results()` — canonical grouping by `(type, inPlace)` + sort by size. +- `convert_to_graph_dict()` — expands each `(type, inPlace)` into its own series; no overwrites. +- `check_bw_dip` / `check_lat_dip` / `check_bus_bw` — group + sort before comparing. + +Comparing **like-for-like on the full key** is a prerequisite for any verdict to be +meaningful, and is the foundation the A/B detector builds on. + +--- + +## 7. Robustness features + +### Retry transient failures (`ci_robustness_lib.run_with_retries`) +- A sweep that fails transiently (NCCL/MPI bootstrap, network, timeout) is retried + up to `retry.max_retries` with linear backoff. +- **Data-corruption / schema-validation failures are never retried** + (`classify_failure`) — retrying would only hide a genuine bug. +- Retries replace a failed run (they don't add samples), so statistics stay clean. +- Transparent on healthy runs (no behavior change when nothing fails). + +### Kill stale GPU state before launch (`ci_robustness_lib.build_gpu_cleanup_script`) +- Runs **first** (test `test_00_cleanup_stale_gpu_state`) and **between retries**. +- Kills leftover RCCL/MPI processes (`pkill -f`, self-match-safe via the `[x]yz` + trick), optionally GPU-holding PIDs (`rocm-smi --showpids`) and stale + docker/podman containers. Best-effort — never fails the job. +- On exclusively-allocated nodes, any leftover process is stale by definition. + +### Refuse to answer rather than answer wrongly + +The organising rule of the CI wrapper: **0 confirmed regressions is only a PASS if +the detector actually looked.** Every layer that could turn "we did not measure" +into "✅ PASS" has been closed: + +- Each group carries `summary.trustworthy`. A group that tripped the circuit + breaker, exceeded `max_inconclusive_frac`, or had unbalanced A/B sample counts + is not trustworthy. +- The report carries a top-level `trustworthy` and `untrustworthy_reasons`, plus + `groups_scored` / `groups_expected` so a silently-dropped group is visible. +- `format_report.py` renders **⚠️ NO VERDICT (not measured)** and exits **2** for + an untrustworthy report — neither a pass nor a regression. The workflow gate + step propagates exit 2, and the PR comment says so in as many words. +- Reports predating the flag are treated as untrustworthy, not as passes. + +### Transport capability pre-flight + +`run_rccl_ab.sh` checks the A/B transport capabilities before spending an +allocation. A run whose interconnect quietly fell back to a slower path measures +something real, but not the thing the gate is supposed to be gating. + +### Circuit breaker and right-sized timeouts + +`circuit_breaker_failures` (2) abandons a group after consecutive failures instead +of grinding through the whole matrix at the per-collective timeout. +`per_collective_timeout_sec` is 360s, not the 1800s it was: with 8 groups × 7 +repeats × 2 sides, the old value put the all-timeout path well past a working day. +The Slurm wall clock, the poller's run and queue budgets, and the Actions job +timeout are now nested innermost-first so the inner layer always fires first and +gets to say *why* — see the table in `submit_and_poll.sh`. + +### Per-run workspace isolation + +Every run gets its own directory under `runs//`: an immutable `cvs/` +snapshot (so a mid-flight deploy cannot change the code a running job executes), +its own config copy, build output, artifacts and logs. Concurrent runs cannot +read or clobber each other's state, and the report records which detector commit +produced it. A cron janitor reclaims old runs, old build-cache entries and old +Slurm logs, reporting what it kept as well as what it removed. + +--- + +## 8. Architecture / code layout + +Pure decision logic is separated from cluster orchestration so it can be +exhaustively unit-tested on a login node (no GPUs). + +**Test inventory** (all runnable without an allocation): 34 collected pytest cases +across `test_regression_lib.py` + `test_ci_robustness_lib.py`, 8 more in +`ci/rccl_perf_gate/tests/test_regression_lib.py`, and 21 shell assertions across +the two `.sh` tests below. + +| File | Role | +|------|------| +| `cvs/cvs/lib/regression_lib.py` | **Pure** A/B detector: gates, percentiles, threshold derivation, report. | +| `cvs/cvs/lib/ci_robustness_lib.py` | **Pure** retry + GPU-cleanup builders/parsers. | +| `cvs/cvs/lib/rccl_lib.py` | Runs one RCCL sweep (`rccl_regression`), `group_rccl_results`, `cleanup_gpus_on_nodes`. | +| `cvs/cvs/tests/rccl/rccl_ab_regression.py` | Pytest orchestration: cleanup → interleaved A/B sweeps (with retry) → analyze. | +| `cvs/cvs/lib/unittests/test_regression_lib.py` | Detector tests incl. Monte-Carlo FP/detection sweeps. | +| `cvs/cvs/lib/unittests/test_ci_robustness_lib.py` | Retry + cleanup tests. | +| `cvs-sbatch/env/thor_rccl_env.sh` | NCCL/IB transport env (cv350 / MI350X + Broadcom Thor RoCE). | +| `cvs-sbatch/env/ainic_rccl_env.sh` | NCCL/IB transport env (tensorwave / MI355X + AINIC). | +| `cvs-sbatch/config_ab*.json` | A/B run configs. | +| `cvs-sbatch/sbatch/ab_regression.sbatch` | SLURM job (`sp_tests`, 4 nodes / 32 ranks). | +| `cvs-sbatch/run.sh`, `lib/python_env.sh` | Orchestrator: cluster.json gen, per-job uv venv. | + +### The CI wrapper (`cvs/ci/rccl_perf_gate/`) + +Everything the GitHub gate needs that is not detection logic. Added after the +detector itself, and version-controlled here rather than living loose on NFS. + +| File | Role | +|------|------| +| `submit_and_poll.sh` | Submits `rccl_ab.sbatch`, polls to a terminal state, maps the job's exit code to the step's. Owns the run/queue budgets and a `scancel` trap so a killed step never orphans an allocation. | +| `format_report.py` | Renders the PR comment. Exit **0** pass / **1** regression / **2** NO VERDICT. | +| `sbatch/rccl_ab.sbatch` | Detect job: 4 pinned nodes, `--exclusive`, 4h wall clock. | +| `sbatch/rccl_build.sbatch` | Build job: single node, CPU compile. Deliberately **not** in the detect reservation — see below. | +| `sbatch/run_rccl_ab.sh` | In-allocation orchestration: transport pre-flight, workspace setup, detector invocation, verdict recount. | +| `sbatch/run_rccl_build.sh` | Content-addressed build of reference + candidate `librccl.so`, keyed on git rev + recipe hash. | +| `sbatch/lib/workspace.sh` | Per-run workspace creation, build cache, GC, and the cron janitor. | +| `configs/*.json` | Snapshots of the live NFS configs, so the gate's decision boundary has version history. **NFS is still what runs** — see `configs/README.md`. | +| `tests/test_regression_lib.py` | 8 detector tests aimed at the trustworthiness paths. | +| `tests/test_workspace_gc.sh` | 13 assertions; runs GC against a throwaway root. | +| `tests/test_build_submit_trap.sh` | 8 assertions; kills the build step mid-flight and checks the allocation is released. | +| `tests/lint_workflow.py` | `bash -n` over every `run:` block in the workflow. | + +> **Reservations.** `rccl_ci` is exactly the four pinned detect nodes +> (`mia1-p01-g[22,26,28,32]`) and the detect job takes all four `--exclusive`, so +> it has no slack. The build must therefore stay out of it: it is a single-node +> CPU compile that needs no GPU, and it runs in `rccl_dev` via +> `SLURM_BUILD_RESERVATION`. It used to pin `--nodelist=mia1-p01-g28`, inside the +> detect set, where it queued behind — and delayed — anything using the pool. + +--- + +## 9. Configuration reference (`rccl` block) + +```jsonc +{ + "rccl": { + "mpi_params": { "no_of_nodes": "4", "no_of_local_ranks": "8", "mpi_pml": "ob1", + "mpi_dir": "/apps/sp/ompi-install", "mpi_oob_port": "10.190.162.57/21" }, + "env_source_script": ".../thor_rccl_env.sh", + "rccl_test_params": { "start_msg_size": "1024", "end_msg_size": "4G", "step_function": "2", + "no_of_iterations": "20", "warmup_iterations": "10", ... }, + "cvs_params": { "nic_model": "thor", "verify_bus_bw": "False", ... }, + + "rccl_collective": ["all_reduce_perf", "reduce_scatter_perf", ...], + "data_types": ["float", "bfloat16"], + "regression": { "NCCL_ALGO": ["Ring"], "NCCL_PROTO": ["Simple"], "NCCL_PXN_DISABLE": ["0","1"] }, + + "gpu_cleanup": { "enabled": true, "kill_gpu_pids": true, "kill_containers": false, "use_sudo": false }, + "retry": { "max_retries": 2, "backoff_sec": 15 }, + + "ab_regression": { + "repeats": 7, + "control_mode": false, // true = reference-vs-itself calibration/stability proof + "safety_factor": 2.0, + "mad_k": 3.0, // thresholds = safety_factor x (median + mad_k x MAD) + "thresholds": { "small": 0.15, "mid": 0.08, "large": 0.06 }, + "max_thresholds": { "small": 0.15, "mid": 0.08, "large": 0.06 }, // hand-set ceiling + "tier_boundaries": { "small_max_bytes": 1048576, "mid_max_bytes": 67108864 }, + "adjacency_min_run": 2, + "min_repeats": 2, + "min_bandwidth_floor": { "small": 0.005, "mid": 0.05, "large": 0.5 }, // per tier + + // Trustworthiness gates -- see section 7. + "circuit_breaker_failures": 2, // abandon a group after N consecutive failures + "require_balanced_samples": true, // A and B must have equal surviving repeats + "max_inconclusive_frac": 0.1, // a group that abstained on >10% is untrustworthy + "publish_derived_thresholds": true, // control mode only; withheld if untrustworthy + + // alltoall is excluded: pooling it inflated the derived large-tier threshold + // by 60-100x, blinding the gate for every other collective. Per-collective + // thresholds make re-enabling it possible, but only after a fresh calibration. + "skip_keys": [["alltoall_perf", "float"], ["alltoall_perf", "bfloat16"]], + + "metric": "busBw", "higher_is_better": true, + "output_dir": "/it-share/rccl-ci/runs//artifacts", // per-run, not shared + "reference": { "label": "ref", "rccl_tests_dir": "...", "ld_library_path": "..." }, + "candidate": { "label": "cand", "rccl_tests_dir": "...", "ld_library_path": "..." } + } + } +} +``` + +Notes: +- `librccl.so` is selected automatically by each binary's **rpath** — no + `ld_library_path` needed (but `reference.ld_library_path` / `candidate.ld_library_path` + are supported if a build needs it). +- The test matrix = `rccl_collective` × `data_types` × Cartesian product of `regression` + env vars, each run for A and B × `repeats`. + +--- + +## 10. How to run + +### Local checkout (`/it-share/rccl-ci`) + +Branches: `aimvt-196-rccl-regression-robustness` in both `cvs/` and `cvs-sbatch/`. +Cluster: **amd-tw**, reservation **rccl_dev**, 4 nodes / 32 ranks. + +```bash +# Fast pipeline smoke (control mode, ~30 min): env + orchestration + detector, 0 regressions expected +sbatch /it-share/rccl-ci/sbatch/rccl_ab.sbatch + +# Full-matrix control calibration (reference as both sides). Writes ab_derived_thresholds.json; +# MUST report 0 regressions. +sbatch --export=ALL,CONFIG_JSON=/it-share/rccl-ci/configs/ab_control.json \ + /it-share/rccl-ci/sbatch/rccl_ab.sbatch + +# Real detection (reference vs candidate), using calibrated thresholds. +sbatch --export=ALL,CONFIG_JSON=/it-share/rccl-ci/configs/ab_detect.json \ + /it-share/rccl-ci/sbatch/rccl_ab.sbatch +``` + +**Build paths on this cluster** + +| Side | rccl-tests dir | librccl (via rpath) | +|------|----------------|---------------------| +| reference | `/it-share/rccl-tests/build` | `/it-share/rccl/install/lib` | +| candidate | `/it-share/sp-tests/therock/bin` | `/it-share/sp-tests/therock/lib` | + +**Logs** (under `/it-share/rccl-ci/logs/`): + +- `sp_tests-.out` / `.err` — Slurm capture (tee'd from the job). +- `run__/` — timestamped run bundle: + - `pytest.log` — pytest output + - `slurm.out` / `slurm.err` — copies of the Slurm logs + - `ab_artifacts/` — detector report, thresholds, `rccl_runs.log` +- `latest` — symlink to the most recent `run_*` directory. + +**Artifacts** (also at `ab_artifacts/` during the run): + +- `ab_regression_report.json` — per-key verdicts. +- `ab_derived_thresholds.json` — calibrated thresholds + measured noise (control mode). +- `rccl_runs.log` — clean per-run record: MPI launch command + rccl-tests output. + +Exit code: non-zero (CI fail) if any confirmed regression. + +**Prerequisites on amd-tw** + +- `~/.ssh/cluster_id_ed25519` must exist and authorize SSH to all nodes in the + allocation (auto-detected by `cvs-sbatch/run.sh`). +- RCCL reference/candidate binaries at the paths in `configs/ab_*.json` (see table above). + +### Original cv350 / MI350X cluster (`/apps/sp/AIMVT-196`) + +```bash +# Calibrate + prove stability (reference as both sides). Writes ab_derived_thresholds.json +# and MUST report 0 regressions. +sbatch --export=ALL,CONFIG_JSON=config_ab_full.json \ + /apps/sp/AIMVT-196/cvs-sbatch/sbatch/ab_regression.sbatch # control_mode: true + +# Real detection (reference vs candidate), using calibrated thresholds. +sbatch --export=ALL,CONFIG_JSON=config_ab_full.json \ + /apps/sp/AIMVT-196/cvs-sbatch/sbatch/ab_regression.sbatch # control_mode: false +``` + +- All jobs are named **`sp_tests`**, 4 nodes / 32 ranks, partition `meta64` / `xgmi36`. + +--- + +## 11. Trust model — how we know it's trustworthy + +| Mechanism | What it buys | +|-----------|--------------| +| Paired A/B, interleaved | Cancels common-mode noise; stable even for small messages | +| Triple gate (threshold ∧ separation ∧ adjacency) | A false positive needs three unlikely things at once | +| Median + percentile separation | Resistant to single bad/straggler runs | +| Thresholds from measured noise (`median + 3·MAD`, per collective) | Bar sits above real run-to-run spread, and a few flaky keys cannot move it | +| `max_thresholds` ceiling | Calibration can only tighten the gate, never loosen it into uselessness | +| Per-tier `min_bandwidth_floor` → inconclusive | Abstains on the region where no judgment is safe, *per tier* | +| Correct full-key group-by | Compares like-for-like; no hidden/spurious signals | +| Pure, unit-tested core (42 pytest cases + 21 shell assertions) | Deterministic, auditable, regression-proof logic | +| A=A control = 0 regressions | Empirically measures the false-positive rate on real HW | +| **Trustworthiness propagated to the verdict** | "0 regressions" cannot be reported unless the detector actually measured; otherwise NO VERDICT (exit 2) | +| Calibration withheld from an untrustworthy control run | A control run that failed its own checks cannot poison later detect runs | +| Immutable per-run `cvs/` snapshot + provenance in the report | The verdict names the exact detector commit and both builds that produced it | + +### Evidence collected +- **Monte-Carlo (simulated noise):** 0/400 false positives; 400/400 detection of an + injected 15% regression. +- **Real 4-node MI350X control (A=A):** **0 false positives over 920 keys** + (5 collectives × 2 dtypes × PXN {0,1}). +- **Real candidate detection (7.0.2 vs develop):** 106 confirmed regressions with a + coherent, structured signature (selective per collective + PXN-dependent for + all_gather; alltoall clean) — strong evidence of a real, localized change rather + than noise. See `RCCL_REGRESSION_FINDINGS.md`. + +--- + +## 12. Limitations & future work + +- ~~**Global per-tier thresholds** are set by the noisiest collective.~~ + **Done.** Thresholds are now derived per collective, and the estimator is + `median + k·MAD` rather than `p95`, so one flaky key no longer sets the bar for + its whole tier. `alltoall_perf` remains in `skip_keys` and can be re-enabled + after a fresh calibration. +- **Sub-floor tiny messages (1K–64K)** are `inconclusive` (busBw ≈ 0). A + **latency-based comparison** (`metric: "time"`, already supported by the detector) + would extend trustworthy coverage to the smallest sizes, where latency is the + meaningful quantity and pairs just as well. +- **Retry path** is proven by unit tests; a fault-injection run would also exercise a + real transient retry + cleanup cycle on hardware. +- **Single-node** runs hit an OpenMPI intra-node bootstrap issue on this cluster; the + validated path is multi-node under `sbatch` (which is the CI path anyway). +- Periodic **A=A canary** runs in CI are recommended to continuously confirm the + false-positive rate stays 0 as the cluster/software evolves. +- **`sbatch --wait` instead of polling.** `submit_and_poll.sh` polls `squeue` on a + 30s interval. `--wait` would remove the polling loop, but it also removes the + per-state visibility the budgets are built on (queued-vs-running is what lets + the run budget be charged against run time only). Deliberately deferred. +- **The configs are hand-synced.** The gate loads from `/it-share/rccl-ci/configs/` + on NFS; `ci/rccl_perf_gate/configs/` is a snapshot for history that nothing reads. + Pointing the readers at the repo checkout would close the drift window but + changes where a live gate loads from, so it is a follow-up, not part of a + robustness pass. Diff before trusting either copy. +- **The detect reservation is shared.** `rccl_ci` is four nodes and the detect job + needs all four; a neighbouring single-node pipeline occupying one of them is + enough to stall the gate, and its declared `TimeLimit` (not its real runtime) is + what Slurm's backfill scheduler plans around. Node contention, not detector + runtime, is now the dominant term in end-to-end latency. + +--- + +## 13. One-line summary + +**Trust = paired design to cancel noise + a triple gate and robust statistics to +resist what's left + thresholds calibrated from measured on-hardware noise + an A=A +control that empirically proves zero false positives — all in a pure, unit-tested, +auditable core, wrapped with retry and stale-GPU cleanup for CI resilience — and a +gate that refuses to say PASS when it did not actually measure.** diff --git a/ci/rccl_perf_gate/README.md b/ci/rccl_perf_gate/README.md new file mode 100644 index 000000000..502cb9a7e --- /dev/null +++ b/ci/rccl_perf_gate/README.md @@ -0,0 +1,27 @@ +# rccl_perf_gate + +Slurm submission/polling/reporting glue for the RCCL paired A/B performance +regression gate used by `ROCm/rocm-systems`'s +[`rccl_perf_regression.yml`](https://github.com/ROCm/rocm-systems/blob/main/.github/workflows/rccl_perf_regression.yml) +GitHub Actions workflow. + +The workflow's self-hosted runner invokes these scripts directly: + +- `sbatch/rccl_build.sbatch`, `sbatch/run_rccl_build.sh` — build RCCL (via + `cvs-sbatch`) as a Slurm job. +- `submit_and_poll.sh`, `sbatch/rccl_ab.sbatch`, `sbatch/run_rccl_ab.sh` — + submit the paired A/B regression job (`cvs/tests/rccl/rccl_ab_regression.py`), + poll it to completion, and map its exit code to a CI-gatable result. +- `format_report.py` — render the A/B run's JSON result into a Markdown + summary for the workflow's job summary / PR comment. + +All scripts honor an `RCCL_CI_ROOT` env override (default `/it-share/rccl-ci`) +so they aren't tied to one cluster's NFS layout. + +## Status + +This is a stopgap. It exists because CVS does not yet submit and manage Slurm +(or Kubernetes) jobs natively — these scripts are thin bash wrappers around +`sbatch`/`squeue` bridging that gap. Once CVS gains native scheduler +integration, this directory should be retired in favor of driving the A/B +regression test directly through CVS. diff --git a/ci/rccl_perf_gate/configs/README.md b/ci/rccl_perf_gate/configs/README.md new file mode 100644 index 000000000..ffd0431a8 --- /dev/null +++ b/ci/rccl_perf_gate/configs/README.md @@ -0,0 +1,51 @@ +# Perf-gate configs + +These are the configs that decide what the gate measures and what counts as a +regression. They used to exist **only** at `/it-share/rccl-ci/configs/` on NFS, +which meant the gate's decision boundary had no history: a threshold could be +widened by hand at 2am and nothing would record that it happened, who did it, or +what it was before. A green check is only as trustworthy as the numbers behind +it, so those numbers are now version-controlled. + +## Which file does what + +| file | used by | role | +|---|---|---| +| `ci_detect_prod.json` | the PR gate (`rccl_perf_regression.yml`, default `config` input) | reference-vs-candidate detection: test matrix, repeats, thresholds, timeouts | +| `ci_control.json` | calibration / control (A=A) runs | derives the noise floor that the detect thresholds are set against | + +## NFS is still the live copy + +Nothing reads from this directory at runtime. `/it-share/rccl-ci/configs/` remains +the deployment target, because the workflow, the sbatch scripts and hand-run +`workflow_dispatch` invocations all pass absolute paths into it. + +So this directory is a **source of truth that must be kept in sync by hand**: + +```bash +# after editing a config here +scp cvs/ci/rccl_perf_gate/configs/ci_detect_prod.json \ + tensorwave-slurm-rccl:/it-share/rccl-ci/configs/ci_detect_prod.json + +# to check for drift +ssh tensorwave-slurm-rccl 'md5sum /it-share/rccl-ci/configs/ci_detect_prod.json' +md5sum cvs/ci/rccl_perf_gate/configs/ci_detect_prod.json +``` + +Wiring the readers to pull straight from the repo checkout would remove the +manual step, but it changes where a live gate loads its config from, so it is +deliberately left as a follow-up rather than folded into a robustness pass. + +## Editing thresholds + +Don't hand-tune `thresholds`. Run a control (A=A) calibration, which writes +`configs/ab_derived_thresholds.json` via `median + k*MAD`, and let the detector +pick it up. `max_thresholds` is the ceiling that stops a noisy calibration from +loosening the gate into uselessness — that one is a policy decision and *is* +meant to be edited by hand. + +`_comment` fields inside the configs record why individual collectives are +skipped. Read them before re-enabling anything: `alltoall_perf` is excluded +because pooling it inflated the derived large-tier threshold by 60-100x, which +would have blinded the gate for every other collective. Per-collective +thresholds now make re-enabling it possible, but only after a fresh calibration. diff --git a/ci/rccl_perf_gate/configs/ci_control.json b/ci/rccl_perf_gate/configs/ci_control.json new file mode 100644 index 000000000..d90d0e4b4 --- /dev/null +++ b/ci/rccl_perf_gate/configs/ci_control.json @@ -0,0 +1,102 @@ +{ + "_comment": "Control (A=A) calibration counterpart to ci_detect_prod.json. Everything that affects measurement -- size sweep, collectives, skip_keys, repeats, timeouts, min_bandwidth_floor -- MUST match ci_detect_prod.json, otherwise the derived thresholds describe a different experiment than the one being gated. min_bandwidth_floor in particular: a scalar 0.5 GB/s floor here would leave the small tier calibrated on nothing while detect measures it. Thresholds derived by this run are published to configs/ab_derived_thresholds.json only if the run clears its own trustworthiness checks. | RCCL CI calibration / A=A control. Derives ab_derived_thresholds.json and MUST report 0 regressions. Both ref+cand point at builds/_fixed/lib (rebuilt 2026-07-14 from rocm-systems develop HEAD 8a7d08d925, which fixes the gfx950 DDA alltoall segfault, the large-message (>=1G) alltoall OOB/abort/hang, and the alltoall bfloat16 correctness bug -- see CI_PIPELINE_PLAN.md caveats #1/#1b; confirmed via node_health_test.sh smallmsg+largemsg+bf16 sweeps, 0 wrong across all). RCCL_DDA_ENABLE override REMOVED from ainic_rccl_env.sh (RCCL default DdaEnable=1 applies). alltoall_perf re-enabled (removed from skip_keys). Recalibrate on any HW/ROCm/RCCL-build/matrix/node/env change. | UPDATE 2026-07-13: alltoall correctness bugs (DDA segfault c59673fc, bf16 correctness, large-msg >=1G OOB/abort/hang) confirmed FIXED on develop HEAD 8a7d08d925 -- lib rebuilt/repinned, RCCL_DDA_ENABLE override removed from ainic_rccl_env.sh entirely. 2026-07-13 (superseded, kept for the audit trail): control-mode calibration job 12389 measured ~5-6x A=A swings for alltoall_perf in the 128MB-4G band, unique among the 5 gated collectives, at a boundary aligning with the 64MB DDA_THRESHOLD. With a single shared large-tier threshold that inflated the derived value ~60-100x (0.126 -> 8.41), which would have blinded the gate for ALL 5 collectives, so alltoall was skip_keys'd out of comparison pending root-cause or per-collective thresholds. Note the claim in that entry that it 'stays in rccl_collective (runs, for visibility)' was never true: skip_keys is a pytest.skip, so the sweep did not execute at all and no data was archived. || UPDATE 2026-08-12: alltoall_perf RE-ENABLED (skip_keys emptied). Both halves of the 2026-07-13 rationale have changed. (a) The unblocking condition it named is met: thresholds_by_collective is derived per collective and resolved per collective at detect time, and max_thresholds bounds any one collective's contribution, so a noisy collective can no longer widen the gate for the others. The only pooled value alltoall still moves is __default__ (large 0.0300 -> 0.0498), which no gated collective reads while all 5 have their own entry. (b) The instability itself does not reproduce: job 16368 (2026-08-12, 4 nodes, 7 repeats, 10 groups, 460 keys) measured the 128MB-4G band at max/min 1.08-1.14 per size with cv_median 0.022, against the ~5-6x reported in July, and returned 0 A=A false positives -- also 0 when the three gates were replayed offline against the per-collective table production actually uses. alltoall is no longer the noisiest gated collective: broadcast_perf-d=float has more >2x keys (3) than alltoall (1) and a higher small-tier cv_median (0.085 vs 0.031). What changed between July and August is not isolated here; the SDK dist was normalised (DMA-BUF symlink chain restored) in that window, which is a plausible but unproven cause. CAVEAT: AllToAll mid and large derive above the max_thresholds ceiling (0.0927 vs 0.08, 0.0748 vs 0.06), so calibration cannot widen them further. If its noise grows the gate will emit false positives rather than silently pass regressions -- the right failure direction, but re-exclude if that starts happening. Worst A=A rel_drop measured was 0.0161 mid / 0.0165 large, so there is real margin today.", + "rccl": { + "mpi_params": { + "no_of_nodes": "4", + "no_of_local_ranks": "8", + "mpi_pml": "ob1", + "mpi_dir": "/it-share/ompi-5.0.8", + "mpi_oob_port": "eno0", + "ucx_tls": "tcp" + }, + "env_source_script": "/it-share/rccl-ci/cvs-sbatch/env/ainic_rccl_env.sh", + "rccl_test_params": { + "rccl_tests_dir": "/it-share/rccl-ci/rccl-tests-2.30.4/bin", + "start_msg_size": "1024", + "end_msg_size": "4G", + "step_function": "2", + "threads_per_gpu": "1", + "warmup_iterations": "10", + "no_of_iterations": "20", + "no_of_cycles": "1", + "check_iteration_count": "1", + "rccl_timeout": "360", + "per_collective_timeout_sec": 360, + "output_algo_proto_channels": false + }, + "cvs_params": { + "cluster_snapshot_debug": "False", + "nic_model": "ainic", + "verify_bus_bw": "False", + "verify_bw_dip": "False", + "verify_lat_dip": "False", + "cvs_exec_timeout": "7200", + "rccl_result_file": "/tmp/rccl_ci_control.json" + }, + "gpu_cleanup": { + "enabled": true, + "kill_gpu_pids": true, + "kill_containers": false, + "use_sudo": false + }, + "retry": { + "max_retries": 2, + "backoff_sec": 15 + }, + "rccl_collective": [ + "all_reduce_perf", + "reduce_scatter_perf", + "all_gather_perf", + "broadcast_perf", + "alltoall_perf" + ], + "data_types": [ + "float", + "bfloat16" + ], + "ab_regression": { + "repeats": 7, + "control_mode": true, + "skip_keys": [], + "safety_factor": 2.0, + "adjacency_min_run": 2, + "min_repeats": 2, + "min_bandwidth_floor": { + "small": 0.005, + "mid": 0.05, + "large": 0.5 + }, + "metric": "busBw", + "higher_is_better": true, + "thresholds": { + "small": 0.15, + "mid": 0.08, + "large": 0.06 + }, + "tier_boundaries": { + "small_max_bytes": 1048576, + "mid_max_bytes": 67108864 + }, + "output_dir": "/it-share/rccl-ci/ab_artifacts", + "reference": { + "label": "ref", + "rccl_tests_dir": "/it-share/rccl-ci/rccl-tests-2.30.4/bin", + "ld_library_path": "/it-share/rccl-ci/builds/_fixed/lib:/it-share/ompi-5.0.8/lib:/it-share/rccl-ci/rocm_devel/lib:/it-share/rccl-ci/lib/libionic" + }, + "candidate": { + "label": "cand", + "rccl_tests_dir": "/it-share/rccl-ci/rccl-tests-2.30.4/bin", + "ld_library_path": "/it-share/rccl-ci/builds/_fixed/lib:/it-share/ompi-5.0.8/lib:/it-share/rccl-ci/rocm_devel/lib:/it-share/rccl-ci/lib/libionic" + }, + "max_thresholds": { + "small": 0.15, + "mid": 0.08, + "large": 0.06 + }, + "mad_k": 3.0, + "circuit_breaker_failures": 2, + "require_balanced_samples": true, + "max_inconclusive_frac": 0.1 + } + } +} diff --git a/ci/rccl_perf_gate/configs/ci_detect_prod.json b/ci/rccl_perf_gate/configs/ci_detect_prod.json new file mode 100644 index 000000000..0a070abc6 --- /dev/null +++ b/ci/rccl_perf_gate/configs/ci_detect_prod.json @@ -0,0 +1,102 @@ +{ + "_comment": "RCCL per-PR perf-regression detection. Ref+cand both use _fixed/lib (rebuilt 2026-07-14 from rocm-systems develop HEAD 8a7d08d925 -- fixes gfx950 DDA alltoall segfault, large-message (>=1G) alltoall OOB/abort/hang, and alltoall bfloat16 correctness bug). BUILD_RCCL=1 overwrites reference/candidate in-allocation for PRs touching projects/rccl. alltoall_perf re-enabled (removed from skip_keys); RCCL_DDA_ENABLE override removed from ainic_rccl_env.sh. | UPDATE 2026-07-13: alltoall correctness bugs (DDA segfault c59673fc, bf16 correctness, large-msg >=1G OOB/abort/hang) confirmed FIXED on develop HEAD 8a7d08d925 -- lib rebuilt/repinned, RCCL_DDA_ENABLE override removed from ainic_rccl_env.sh entirely. 2026-07-13 (superseded, kept for the audit trail): control-mode calibration job 12389 measured ~5-6x A=A swings for alltoall_perf in the 128MB-4G band, unique among the 5 gated collectives, at a boundary aligning with the 64MB DDA_THRESHOLD. With a single shared large-tier threshold that inflated the derived value ~60-100x (0.126 -> 8.41), which would have blinded the gate for ALL 5 collectives, so alltoall was skip_keys'd out of comparison pending root-cause or per-collective thresholds. Note the claim in that entry that it 'stays in rccl_collective (runs, for visibility)' was never true: skip_keys is a pytest.skip, so the sweep did not execute at all and no data was archived. || UPDATE 2026-08-12: alltoall_perf RE-ENABLED (skip_keys emptied). Both halves of the 2026-07-13 rationale have changed. (a) The unblocking condition it named is met: thresholds_by_collective is derived per collective and resolved per collective at detect time, and max_thresholds bounds any one collective's contribution, so a noisy collective can no longer widen the gate for the others. The only pooled value alltoall still moves is __default__ (large 0.0300 -> 0.0498), which no gated collective reads while all 5 have their own entry. (b) The instability itself does not reproduce: job 16368 (2026-08-12, 4 nodes, 7 repeats, 10 groups, 460 keys) measured the 128MB-4G band at max/min 1.08-1.14 per size with cv_median 0.022, against the ~5-6x reported in July, and returned 0 A=A false positives -- also 0 when the three gates were replayed offline against the per-collective table production actually uses. alltoall is no longer the noisiest gated collective: broadcast_perf-d=float has more >2x keys (3) than alltoall (1) and a higher small-tier cv_median (0.085 vs 0.031). What changed between July and August is not isolated here; the SDK dist was normalised (DMA-BUF symlink chain restored) in that window, which is a plausible but unproven cause. CAVEAT: AllToAll mid and large derive above the max_thresholds ceiling (0.0927 vs 0.08, 0.0748 vs 0.06), so calibration cannot widen them further. If its noise grows the gate will emit false positives rather than silently pass regressions -- the right failure direction, but re-exclude if that starts happening. Worst A=A rel_drop measured was 0.0161 mid / 0.0165 large, so there is real margin today. | Static thresholds fallback refreshed 2026-07-14 from job 12455 control-mode recalibration (4 collectives, alltoall excluded) -- use_derived_thresholds is True by default so ab_derived_thresholds.json is the live source of truth at detect time; this field is only the fallback if that file is ever missing.", + "rccl": { + "mpi_params": { + "no_of_nodes": "4", + "no_of_local_ranks": "8", + "mpi_pml": "ob1", + "mpi_dir": "/it-share/ompi-5.0.8", + "mpi_oob_port": "eno0", + "ucx_tls": "tcp" + }, + "env_source_script": "/it-share/rccl-ci/cvs-sbatch/env/ainic_rccl_env.sh", + "rccl_test_params": { + "rccl_tests_dir": "/it-share/rccl-ci/rccl-tests-2.30.4/bin", + "start_msg_size": "1024", + "end_msg_size": "4G", + "step_function": "2", + "threads_per_gpu": "1", + "warmup_iterations": "10", + "no_of_iterations": "20", + "no_of_cycles": "1", + "check_iteration_count": "1", + "rccl_timeout": "360", + "per_collective_timeout_sec": 360, + "output_algo_proto_channels": false + }, + "cvs_params": { + "cluster_snapshot_debug": "False", + "nic_model": "ainic", + "verify_bus_bw": "False", + "verify_bw_dip": "False", + "verify_lat_dip": "False", + "cvs_exec_timeout": "7200", + "rccl_result_file": "/tmp/rccl_ci_detect.json" + }, + "gpu_cleanup": { + "enabled": true, + "kill_gpu_pids": true, + "kill_containers": false, + "use_sudo": false + }, + "retry": { + "max_retries": 2, + "backoff_sec": 15 + }, + "rccl_collective": [ + "all_reduce_perf", + "reduce_scatter_perf", + "all_gather_perf", + "broadcast_perf", + "alltoall_perf" + ], + "data_types": [ + "float", + "bfloat16" + ], + "ab_regression": { + "repeats": 7, + "control_mode": false, + "skip_keys": [], + "safety_factor": 2.0, + "adjacency_min_run": 2, + "min_repeats": 2, + "min_bandwidth_floor": { + "small": 0.005, + "mid": 0.05, + "large": 0.5 + }, + "metric": "busBw", + "higher_is_better": true, + "thresholds": { + "small": 0.15, + "mid": 0.08, + "large": 0.06 + }, + "tier_boundaries": { + "small_max_bytes": 1048576, + "mid_max_bytes": 67108864 + }, + "output_dir": "/it-share/rccl-ci/ab_artifacts", + "reference": { + "label": "ref", + "rccl_tests_dir": "/it-share/rccl-ci/rccl-tests-2.30.4/bin", + "ld_library_path": "/it-share/rccl-ci/builds/_fixed/lib:/it-share/ompi-5.0.8/lib:/it-share/rccl-ci/rocm_devel/lib:/it-share/rccl-ci/lib/libionic" + }, + "candidate": { + "label": "cand", + "rccl_tests_dir": "/it-share/rccl-ci/rccl-tests-2.30.4/bin", + "ld_library_path": "/it-share/rccl-ci/builds/_fixed/lib:/it-share/ompi-5.0.8/lib:/it-share/rccl-ci/rocm_devel/lib:/it-share/rccl-ci/lib/libionic" + }, + "circuit_breaker_failures": 2, + "max_thresholds": { + "small": 0.15, + "mid": 0.08, + "large": 0.06 + }, + "max_inconclusive_frac": 0.1, + "require_balanced_samples": true, + "mad_k": 3.0 + } + } +} diff --git a/ci/rccl_perf_gate/format_report.py b/ci/rccl_perf_gate/format_report.py new file mode 100644 index 000000000..a22af5991 --- /dev/null +++ b/ci/rccl_perf_gate/format_report.py @@ -0,0 +1,264 @@ +#!/usr/bin/env python3 +"""Render an RCCL A/B regression report (ab_regression_report.json) as Markdown. + +Consumes the JSON written by ``cvs/tests/rccl/rccl_ab_regression.py`` +(``{"control_mode": bool, "reports": {group_key: }}``) +and emits a GitHub-flavoured Markdown summary suitable for a PR comment or a +GitHub Actions step summary. + +Exit code mirrors the gate verdict so the same invocation can drive a check: + 0 = PASS (no confirmed regressions) + 1 = REGRESSION DETECTED (detect mode) / detector unstable (control mode) + 2 = NO VERDICT -- the report is unreadable, or the detector flagged the run as + untrustworthy (incomplete repeats, circuit breaker tripped, A=A in detect + mode, too many inconclusive keys). This is not a PASS. +Use ``--no-exit-code`` to always exit 0 (e.g. when only rendering). +""" + +import argparse +import json +import sys +from pathlib import Path + + +def _fmt_size(n): + """Human-readable byte size (1024-based), e.g. 1024 -> '1K', 4294967296 -> '4G'.""" + try: + n = int(n) + except (TypeError, ValueError): + return str(n) + for unit in ("", "K", "M", "G", "T"): + if abs(n) < 1024 or unit == "T": + return f"{n}{unit}" if unit == "" else f"{n:.0f}{unit}" + n /= 1024.0 + return str(n) + + +def _fmt_tiers(thr): + return ( + f"small {thr.get('small', 0) * 100:.1f}% · " + f"mid {thr.get('mid', 0) * 100:.1f}% · " + f"large {thr.get('large', 0) * 100:.1f}%" + ) + + +def _thresholds_line(reports): + """Pull the thresholds from the first report (identical across groups). + + Handles both shapes the detector emits: a flat per-tier table, and the + per-collective table {collective: {tier: value}} that calibration produces + when collectives differ enough in noise to need their own numbers. + """ + for rep in reports.values(): + thr = rep.get("config", {}).get("thresholds") + if not thr: + continue + if all(isinstance(v, dict) for v in thr.values()): + parts = [f"{name}: {_fmt_tiers(tiers)}" for name, tiers in sorted(thr.items())] + return "
".join(parts) + return _fmt_tiers(thr) + return "n/a" + + +def _trust(report_data): + """(trustworthy, [reasons]) for the run as a whole. + + Falls back to the per-group flags so a report written before the top-level + flag existed still cannot be read as a clean PASS by accident. + """ + reports = report_data.get("reports", {}) or {} + top = report_data.get("trustworthy") + reasons = list(report_data.get("untrustworthy_reasons") or []) + if top is None: + top = bool(reports) and all( + r.get("summary", {}).get("trustworthy", False) for r in reports.values()) + if not top and not reasons: + reasons = ["this report predates the trustworthiness flag, or a group could not be scored"] + return bool(top), reasons + + +def _provenance_line(report_data): + """One line naming exactly what was compared, so a verdict can be traced back.""" + prov = report_data.get("provenance") or {} + bits = [] + sha = prov.get("cvs_sha") + if sha: + bits.append(f"detector `cvs@{str(sha)[:12]}`") + for side in ("reference", "candidate"): + info = prov.get(side) or {} + rev = info.get("built_rev") + if rev: + bits.append(f"{side} `{str(rev)[:12]}`") + for label, key in (("run", "run_key"), ("slurm", "slurm_job_id"), ("gha", "github_run_id")): + val = prov.get(key) + if val: + bits.append(f"{label} `{val}`") + if not bits: + return "**Provenance:** n/a" + return "**Provenance:** " + " · ".join(bits) + + +def _collect(reports): + """Aggregate counts and flatten confirmed regressions across all groups.""" + totals = {"keys": 0, "regressions": 0, "inconclusive": 0, "candidates": 0} + regressions = [] + for group_key, rep in reports.items(): + s = rep.get("summary", {}) + totals["keys"] += s.get("keys_compared", 0) + totals["regressions"] += s.get("regressions", 0) + totals["inconclusive"] += s.get("inconclusive", 0) + totals["candidates"] += s.get("candidates", 0) + for v in rep.get("regressions", []): + k = v.get("key", {}) + regressions.append( + { + "collective": k.get("name", "?"), + "dtype": k.get("type", "?"), + "size": k.get("size", 0), + "a_med": v.get("a", {}).get("median", 0.0), + "b_med": v.get("b", {}).get("median", 0.0), + "drop": v.get("rel_drop", 0.0), + "thr": v.get("threshold", 0.0), + } + ) + regressions.sort(key=lambda r: (r["collective"], str(r["dtype"]), r["size"])) + return totals, regressions + + +def render(report_data, title="RCCL Perf-Regression Gate"): + """Return a Markdown string for the given parsed report JSON.""" + control_mode = bool(report_data.get("control_mode", False)) + reports = report_data.get("reports", {}) + totals, regressions = _collect(reports) + has_regression = totals["regressions"] > 0 + trustworthy, reasons = _trust(report_data) + + lines = [] + if not trustworthy: + # "0 confirmed regressions" only means PASS if the detector actually + # looked. When it didn't, say so instead of rendering a green tick that + # a reviewer will read as "this PR is clean". + lines.append(f"## {title}: ⚠️ NO VERDICT (not measured)") + lines.append("") + lines.append( + "This run did **not** produce a usable answer. It is neither a PASS nor a " + "regression — treat the perf gate as *not run* for this change." + ) + lines.append("") + lines.append("**Why:**") + for reason in reasons or ["(no reason recorded)"]: + lines.append(f"- {reason}") + lines.append("") + elif control_mode: + # In a control (A=A) run, any regression is a false positive => gate broken. + verdict = "❌ DETECTOR UNSTABLE" if has_regression else "✅ STABLE (0 false positives)" + lines.append(f"## {title}: {verdict}") + lines.append("") + lines.append("**Mode:** calibration / control (A=A — same build both sides)") + else: + verdict = "❌ REGRESSION DETECTED" if has_regression else "✅ PASS" + lines.append(f"## {title}: {verdict}") + lines.append("") + lines.append("**Mode:** detect (reference vs candidate)") + + if not trustworthy: + lines.append( + "**Mode:** " + + ("calibration / control (A=A)" if control_mode else "detect (reference vs candidate)") + ) + + lines.append(f"**Thresholds:** {_thresholds_line(reports)}") + source = report_data.get("thresholds_source") + if source: + lines.append(f"**Thresholds source:** `{source}`") + scored = report_data.get("groups_scored") + expected = report_data.get("groups_expected") + coverage = "" + if scored is not None and expected is not None: + coverage = f" · **Groups scored:** {scored}/{expected}" + lines.append( + f"**Keys compared:** {totals['keys']} · " + f"**Confirmed regressions:** {totals['regressions']} · " + f"**Inconclusive:** {totals['inconclusive']}" + coverage + ) + lines.append(_provenance_line(report_data)) + lines.append("") + + if regressions: + lines.append(f"### Confirmed regressions ({len(regressions)})") + lines.append("") + lines.append("| collective | dtype | size | A (ref) GB/s | B (cand) GB/s | drop % | thr % |") + lines.append("|---|---|---:|---:|---:|---:|---:|") + for r in regressions: + lines.append( + f"| {r['collective']} | {r['dtype']} | {_fmt_size(r['size'])} " + f"| {r['a_med']:.2f} | {r['b_med']:.2f} " + f"| {r['drop'] * 100:.1f} | {r['thr'] * 100:.1f} |" + ) + lines.append("") + + # Per-group breakdown (collapsed) so reviewers can see coverage / inconclusive spread. + lines.append("
Per-collective breakdown") + lines.append("") + lines.append("| group | keys | regressions | inconclusive |") + lines.append("|---|---:|---:|---:|") + for group_key, rep in sorted(reports.items()): + s = rep.get("summary", {}) + lines.append( + f"| {group_key} | {s.get('keys_compared', 0)} " + f"| {s.get('regressions', 0)} | {s.get('inconclusive', 0)} |" + ) + lines.append("") + lines.append("
") + lines.append("") + return "\n".join(lines), has_regression, control_mode + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "report", + nargs="?", + default="/it-share/rccl-ci/ab_artifacts/ab_regression_report.json", + help="Path to ab_regression_report.json", + ) + parser.add_argument("-o", "--output", help="Write Markdown here (default: stdout)") + parser.add_argument("--title", default="RCCL Perf-Regression Gate", help="Heading title") + parser.add_argument( + "--no-exit-code", + action="store_true", + help="Always exit 0 (do not map verdict to exit code)", + ) + args = parser.parse_args(argv) + + path = Path(args.report) + try: + report_data = json.loads(path.read_text()) + except FileNotFoundError: + print(f"error: report not found: {path}", file=sys.stderr) + return 2 + except ValueError as exc: + print(f"error: could not parse {path}: {exc}", file=sys.stderr) + return 2 + + markdown, has_regression, _control = render(report_data, title=args.title) + + if args.output: + Path(args.output).write_text(markdown) + else: + print(markdown) + + if args.no_exit_code: + return 0 + trustworthy, reasons = _trust(report_data) + if not trustworthy: + # Distinct from 1 (a real regression) so callers can tell "the gate says + # no" apart from "the gate never got an answer". + for reason in reasons: + print(f"no-verdict: {reason}", file=sys.stderr) + return 2 + return 1 if has_regression else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/ci/rccl_perf_gate/sbatch/lib/check_dmabuf.sh b/ci/rccl_perf_gate/sbatch/lib/check_dmabuf.sh new file mode 100755 index 000000000..fb0d7a36c --- /dev/null +++ b/ci/rccl_perf_gate/sbatch/lib/check_dmabuf.sh @@ -0,0 +1,189 @@ +#!/usr/bin/env bash +# Assert that RCCL resolves ROCr correctly and that BOTH A/B sides agree on the +# transport capabilities they will use. +# +# WHY +# --- +# RCCL probes ROCr at init via dlopen("libhsa-runtime64.so"). If the ROCm dist +# has a flattened layout (several inodes for one SONAME — see +# normalize_rocm_dist.sh), that dlopen returns a SECOND, uninitialised copy of +# ROCr. hsa_system_get_info then returns 4107, RCCL jumps to its error path, +# pfn_hsa_amd_portable_export_dmabuf is never resolved, and DMA-BUF export is +# silently disabled. Multi-node collectives hang for the full rccl_timeout. +# +# Whether a given librccl is affected depends on how it was BUILT: a build that +# links ROCr via DT_NEEDED is immune, one that relies on dlopen is not. That is +# the dangerous part for an A/B gate — reference and candidate can differ. When +# reference has DMA-BUF off and candidate has it on, the comparison is not +# merely slow, it is INVALID: the candidate scores as a huge fake improvement. +# +# So this script checks two things: +# 1. liveness — each side actually has DMA-BUF enabled +# 2. symmetry — both sides resolved the SAME capabilities, so the A/B +# measurement is comparing library changes and nothing else +# +# USAGE +# check_dmabuf.sh --lib DIR # probe one side by lib directory +# check_dmabuf.sh --ldpath 'A:B:C' # probe one side by full LD_LIBRARY_PATH +# check_dmabuf.sh --config ci_detect.json # probe reference AND candidate, compare +# +# Requires GPUs: run inside an allocation with a GRES request (e.g. +# --exclusive --gres=gpu:8). Without a gres request Slurm's device cgroup hides +# /dev/kfd and the probe reports "no ROCm-capable device is detected". +# +# Exit 0 = healthy (and symmetric, in --config mode) +# 1 = DMA-BUF disabled on a side, or the two sides disagree +# 2 = inconclusive / harness failure + +set -uo pipefail + +RCCL_CI_ROOT="${RCCL_CI_ROOT:-/it-share/rccl-ci}" +ROCM_DIST="${ROCM_DIST:-${RCCL_CI_ROOT}/rocm_devel}" +PERF_BIN="${RCCL_CI_ROOT}/rccl-tests-2.30.4/bin/all_reduce_perf" +NGPU="${DMABUF_CHECK_NGPU:-2}" + +LIB_DIR=""; LD_PATH=""; CONFIG=""; ROCR_PATH="" + +while [[ $# -gt 0 ]]; do + case "$1" in + --lib) LIB_DIR="$2"; shift 2 ;; + --ldpath) LD_PATH="$2"; shift 2 ;; + --config) CONFIG="$2"; shift 2 ;; + --rocr-path) ROCR_PATH="$2"; shift 2 ;; + --ngpu) NGPU="$2"; shift 2 ;; + -h|--help) sed -n '2,40p' "$0"; exit 0 ;; + *) echo "[ERROR] unknown arg: $1" >&2; exit 2 ;; + esac +done + +[[ -x "${PERF_BIN}" ]] || { echo "[ERROR] missing ${PERF_BIN}" >&2; exit 2; } + +# --- probe one side ----------------------------------------------------------- +# Echoes " " on stdout; +# human-readable detail goes to stderr so callers can capture the verdict alone. +# dmabuf is one of: enabled | disabled | unknown +probe_side() { + local label="$1" ldpath="$2" + local log resolved hsa_needed dmabuf rocr rc + + log="$(mktemp)" + + resolved="$(LD_LIBRARY_PATH="${ldpath}" ldd "${PERF_BIN}" 2>/dev/null | awk '/librccl/ {print $3}')" + hsa_needed="$(readelf -d "${resolved}" 2>/dev/null | grep -c 'hsa-runtime')" + [[ -z "${hsa_needed}" ]] && hsa_needed=0 + + { + echo "--- ${label} ---" + echo " LD_LIBRARY_PATH : ${ldpath}" + echo " resolved librccl: ${resolved:-}" + echo " hsa DT_NEEDED : ${hsa_needed} (0 = dlopen path, sensitive to dist layout)" + } >&2 + + env -i \ + PATH="/usr/bin:/bin" \ + HOME="${HOME}" \ + LD_LIBRARY_PATH="${ldpath}" \ + NCCL_DEBUG=INFO \ + NCCL_DEBUG_SUBSYS=INIT \ + HSA_NO_SCRATCH_RECLAIM=1 \ + NCCL_IGNORE_CPU_AFFINITY=1 \ + ${ROCR_PATH:+RCCL_ROCR_PATH="${ROCR_PATH}"} \ + timeout 180s "${PERF_BIN}" -b 8 -e 8 -f 2 -g "${NGPU}" > "${log}" 2>&1 + rc=$? + + rocr="$(grep -oiE 'ROCr version [0-9.]+' "${log}" | head -1 | awk '{print $3}')" + [[ -z "${rocr}" ]] && rocr="none" + + if grep -qi 'DMA_BUF Support Enabled' "${log}"; then + dmabuf="enabled" + elif grep -qE '4107|DMA_BUF Support Disabled|Could not find .*dmabuf' "${log}"; then + dmabuf="disabled" + else + dmabuf="unknown" + { echo " [!] no ROCr verdict (perf exit=${rc}); tail:"; tail -12 "${log}" | sed 's/^/ /'; } >&2 + fi + + grep -iE 'rocr|dma.?buf|4107' "${log}" | sed 's/^/ /' >&2 + echo " => dmabuf=${dmabuf} rocr=${rocr}" >&2 + echo >&2 + + rm -f "${log}" + echo "${dmabuf} ${rocr} ${hsa_needed} ${resolved:-none}" +} + +echo "==========================================================================" +echo "RCCL transport capability preflight" +echo " node : $(hostname)" +echo " rocm dist : ${ROCM_DIST}" +echo " rocr path : ${ROCR_PATH:-}" +echo "==========================================================================" + +# --- single-side mode --------------------------------------------------------- +if [[ -z "${CONFIG}" ]]; then + if [[ -n "${LIB_DIR}" ]]; then + LD_PATH="${LIB_DIR}:${ROCM_DIST}/lib:/it-share/ompi-5.0.8/lib" + fi + [[ -n "${LD_PATH}" ]] || { echo "[ERROR] need --lib, --ldpath or --config" >&2; exit 2; } + + read -r dmabuf rocr _hsa _lib <<< "$(probe_side "side" "${LD_PATH}")" + case "${dmabuf}" in + enabled) echo "[PASS] DMA-BUF enabled (ROCr ${rocr})."; exit 0 ;; + disabled) echo "[FAIL] DMA-BUF DISABLED — RCCL loaded a second, uninitialised ROCr." >&2 + echo " Check dist layout: ${RCCL_CI_ROOT}/sbatch/lib/normalize_rocm_dist.sh --check" >&2 + exit 1 ;; + *) echo "[INCONCLUSIVE] no ROCr verdict." >&2; exit 2 ;; + esac +fi + +# --- A/B mode: probe both sides and compare ----------------------------------- +[[ -f "${CONFIG}" ]] || { echo "[ERROR] config not found: ${CONFIG}" >&2; exit 2; } + +read -r REF_LD CAND_LD <<< "$(python3 -c " +import json,sys +d=json.load(open('${CONFIG}')) +ab=d.get('rccl',{}).get('ab_regression',{}) +r=ab.get('reference',{}).get('ld_library_path','') +c=ab.get('candidate',{}).get('ld_library_path','') +if not r or not c: sys.exit(3) +print(r,c) +" 2>/dev/null)" || { echo "[ERROR] could not read ld_library_path for both sides from ${CONFIG}" >&2; exit 2; } + +read -r R_DMABUF R_ROCR R_HSA R_LIB <<< "$(probe_side "reference" "${REF_LD}")" +read -r C_DMABUF C_ROCR C_HSA C_LIB <<< "$(probe_side "candidate" "${CAND_LD}")" + +echo "=== capability summary ===" +printf ' %-10s dmabuf=%-9s rocr=%-6s hsa_needed=%s\n' "reference" "${R_DMABUF}" "${R_ROCR}" "${R_HSA}" +printf ' %-10s dmabuf=%-9s rocr=%-6s hsa_needed=%s\n' "candidate" "${C_DMABUF}" "${C_ROCR}" "${C_HSA}" +echo + +status=0 + +# 1. Liveness. A side without DMA-BUF will hang the multi-node collectives. +for side in reference candidate; do + v="R_DMABUF"; [[ "${side}" == "candidate" ]] && v="C_DMABUF" + case "${!v}" in + enabled) ;; + disabled) + echo "[FAIL] ${side}: DMA-BUF is DISABLED — multi-node collectives will hang." >&2 + echo " Fix the dist layout: ${RCCL_CI_ROOT}/sbatch/lib/normalize_rocm_dist.sh --check" >&2 + status=1 ;; + *) + echo "[WARN] ${side}: capability probe inconclusive." >&2 + [[ ${status} -eq 0 ]] && status=2 ;; + esac +done + +# 2. Symmetry. This is the correctness gate: differing capabilities mean the A/B +# result measures the environment, not the code change under test. +if [[ "${R_DMABUF}" != "${C_DMABUF}" ]]; then + echo "[FAIL] A/B ASYMMETRY: reference dmabuf=${R_DMABUF} but candidate dmabuf=${C_DMABUF}." >&2 + echo " The two sides would not use the same transport path, so any" >&2 + echo " measured delta reflects the environment, not the code change." >&2 + echo " Refusing to report a verdict from an invalid comparison." >&2 + status=1 +fi + +if [[ ${status} -eq 0 ]]; then + echo "[PASS] both sides: DMA-BUF enabled, ROCr ${R_ROCR} — capabilities symmetric." +fi +exit "${status}" diff --git a/ci/rccl_perf_gate/sbatch/lib/normalize_rocm_dist.sh b/ci/rccl_perf_gate/sbatch/lib/normalize_rocm_dist.sh new file mode 100755 index 000000000..cbafce04d --- /dev/null +++ b/ci/rccl_perf_gate/sbatch/lib/normalize_rocm_dist.sh @@ -0,0 +1,228 @@ +#!/usr/bin/env bash +# Normalise a ROCm SDK lib/ directory so each SONAME resolves to exactly ONE inode. +# +# WHY THIS EXISTS +# --------------- +# rocm_devel is a pip-installed wheel (rocm_sdk_devel-*.whl). Wheels are zip +# archives, and the packaging step flattened most versioned-library symlink +# chains into independent regular files. The wheel RECORD confirms it: +# +# _rocm_sdk_devel/lib/libhsa-runtime64.so,, +# _rocm_sdk_devel/lib/libhsa-runtime64.so.1,, +# _rocm_sdk_devel/lib/libhsa-runtime64.so.1.21.0,, +# +# Three separate entries -> three regular files -> three DIFFERENT INODES with +# byte-identical content and the same SONAME. +# +# That breaks a load-bearing glibc invariant: the dynamic loader dedups already +# loaded shared objects by (device, inode), NOT by filename or SONAME. So a +# process that links libhsa-runtime64.so.1 via DT_NEEDED and later +# dlopen()s "libhsa-runtime64.so" gets a SECOND, INDEPENDENT, NEVER-INITIALISED +# copy of ROCr. Every hsa_* call on that handle returns 4107 +# (HSA_STATUS_ERROR_NOT_INITIALIZED). +# +# In RCCL that path is rocmwrap.cc: the dlopen fails its version probe, jumps to +# `error:`, and pfn_hsa_amd_portable_export_dmabuf is never resolved -- DMA-BUF +# is silently disabled, with no message unless NCCL_DEBUG>=WARN. Multi-node +# collectives then fall back to a path that hangs on this fabric. +# +# Reinstalling or rebuilding the SDK does NOT fix this: the same wheel produces +# the same flattened layout. The fix belongs here, as a post-install step that +# runs every time the dist is refreshed. +# +# WHAT IT DOES +# ------------ +# For each group of byte-identical regular files whose names form a versioned +# chain (libfoo.so, libfoo.so.1, libfoo.so.1.2.3), keep the most-versioned file +# as the single real object and replace the shorter names with relative symlinks +# forming the conventional chain: +# +# libfoo.so -> libfoo.so.1 -> libfoo.so.1.2.3 (one inode) +# +# This is exactly the layout the ROCm .deb/.tar ships and what ldconfig would +# produce. It is content-preserving: no bytes change, so processes holding the +# old inodes open are unaffected. +# +# USAGE +# ----- +# normalize_rocm_dist.sh # dry run against the default dist +# normalize_rocm_dist.sh --apply # make the changes (writes a rollback script) +# normalize_rocm_dist.sh --check # invariant assertion; exit 1 if violated +# normalize_rocm_dist.sh --dist /path/to/rocm_devel [--apply|--check] +# +# --check is the CI-facing mode: cheap, read-only, and fails the build with a +# clear message instead of letting a crippled SDK produce a 2h39m hang. + +set -euo pipefail + +DIST="${ROCM_DIST:-/it-share/rccl-ci/rocm_devel}" +MODE="dry-run" + +while [[ $# -gt 0 ]]; do + case "$1" in + --apply) MODE="apply"; shift ;; + --check) MODE="check"; shift ;; + --dry-run) MODE="dry-run"; shift ;; + --dist) DIST="$2"; shift 2 ;; + -h|--help) sed -n '2,50p' "$0"; exit 0 ;; + *) echo "[ERROR] unknown argument: $1" >&2; exit 2 ;; + esac +done + +LIBDIR="${DIST%/}/lib" +[[ -d "${LIBDIR}" ]] || { echo "[ERROR] not a directory: ${LIBDIR}" >&2; exit 2; } + +# Resolve so the rollback script and log are unambiguous even if DIST is a symlink. +REAL_LIBDIR="$(readlink -f "${LIBDIR}")" + +echo "==========================================================================" +echo "ROCm dist SONAME normalisation" +echo " dist : ${DIST}" +echo " lib dir : ${REAL_LIBDIR}" +echo " mode : ${MODE}" +echo "==========================================================================" + +# --- discover groups of content-identical regular .so files ------------------- +# Only regular files: existing symlinks are already correct and must be left alone. +tmp_hashes="$(mktemp)" +trap 'rm -f "${tmp_hashes}"' EXIT + +find "${REAL_LIBDIR}" -maxdepth 1 -type f -name '*.so*' -exec md5sum {} + 2>/dev/null \ + | sed "s| ${REAL_LIBDIR}/| |" > "${tmp_hashes}" + +# Group by hash, emit "hash name1 name2 ..." for groups with >1 member. +groups="$(awk '{h=$1; sub(/^ +/,"",$2); g[h]=g[h]" "$2; n[h]++} + END {for (k in n) if (n[k]>1) print k g[k]}' "${tmp_hashes}" | sort)" + +if [[ -z "${groups}" ]]; then + echo "[OK] no duplicate-inode SONAME groups found — dist is already normalised." + exit 0 +fi + +# --- plan --------------------------------------------------------------------- +declare -a PLAN_LINK PLAN_TARGET +skipped=0 +bytes_freed=0 +groups_ok=0 + +while read -r _hash rest; do + # shellcheck disable=SC2206 + members=( ${rest} ) + + # Sort by name length ascending: libfoo.so, libfoo.so.1, libfoo.so.1.2.3 + mapfile -t sorted < <(printf '%s\n' "${members[@]}" | awk '{print length, $0}' | sort -n | cut -d' ' -f2-) + + # SAFETY GUARD: only touch a group whose names form a genuine versioned chain, + # i.e. each shorter name is a literal prefix of the next longer one. Two + # unrelated libraries that happen to be byte-identical (vendored duplicates, + # stubs) must NOT be collapsed into each other — that would silently rewrite + # the dependency graph. + chain_ok=1 + for (( i = 0; i < ${#sorted[@]} - 1; i++ )); do + if [[ "${sorted[i+1]}" != "${sorted[i]}"* ]]; then chain_ok=0; break; fi + done + + if [[ ${chain_ok} -eq 0 ]]; then + echo "[SKIP] not a versioned chain, leaving alone: ${sorted[*]}" + skipped=$(( skipped + 1 )) + continue + fi + + groups_ok=$(( groups_ok + 1 )) + canonical="${sorted[-1]}" + + # Build the conventional chain: each name points at the next longer name. + for (( i = 0; i < ${#sorted[@]} - 1; i++ )); do + PLAN_LINK+=( "${sorted[i]}" ) + PLAN_TARGET+=( "${sorted[i+1]}" ) + sz="$(stat -c %s "${REAL_LIBDIR}/${sorted[i]}" 2>/dev/null || echo 0)" + bytes_freed=$(( bytes_freed + sz )) + done + + printf ' %-42s <- %s\n' "${canonical}" "$(printf '%s ' "${sorted[@]::${#sorted[@]}-1}")" +done <<< "${groups}" + +echo +echo " chains to normalise : ${groups_ok}" +echo " files -> symlinks : ${#PLAN_LINK[@]}" +echo " groups skipped : ${skipped}" +printf " disk reclaimed : %.2f GB\n" "$(awk -v b="${bytes_freed}" 'BEGIN{print b/1024/1024/1024}')" +echo + +# --- check mode: assert the invariant, change nothing ------------------------- +if [[ "${MODE}" == "check" ]]; then + if [[ ${#PLAN_LINK[@]} -gt 0 ]]; then + echo "[FAIL] ${#PLAN_LINK[@]} duplicate-inode library file(s) in ${REAL_LIBDIR}." >&2 + echo " A dlopen() by unversioned name will load a SECOND, uninitialised" >&2 + echo " copy of these libraries. For ROCr this silently disables DMA-BUF" >&2 + echo " and hangs multi-node collectives." >&2 + echo " Fix: ${BASH_SOURCE[0]} --dist ${DIST} --apply" >&2 + exit 1 + fi + echo "[OK] invariant holds: one inode per SONAME." + exit 0 +fi + +if [[ "${MODE}" == "dry-run" ]]; then + echo "[DRY RUN] nothing changed. Re-run with --apply to make these changes." + exit 0 +fi + +# --- apply -------------------------------------------------------------------- +[[ -w "${REAL_LIBDIR}" ]] || { echo "[ERROR] ${REAL_LIBDIR} is not writable" >&2; exit 1; } + +stamp="$(date +%Y%m%d_%H%M%S)" +rollback="${REAL_LIBDIR}/.normalize_rollback_${stamp}.sh" +{ + echo "#!/usr/bin/env bash" + echo "# Undo normalize_rocm_dist.sh run of ${stamp}." + echo "# Replaces each symlink with an independent copy of its target, restoring" + echo "# the original (broken) multi-inode layout." + echo "set -euo pipefail" + echo "cd \"${REAL_LIBDIR}\"" +} > "${rollback}" + +converted=0 +for (( i = 0; i < ${#PLAN_LINK[@]}; i++ )); do + link="${PLAN_LINK[i]}" + target="${PLAN_TARGET[i]}" + + # Re-verify identical content at apply time. The plan was computed from a + # snapshot; refuse to act on anything that changed underneath us. + if ! cmp -s "${REAL_LIBDIR}/${link}" "${REAL_LIBDIR}/${target}"; then + echo "[SKIP] content diverged since planning: ${link}" >&2 + continue + fi + + echo "cp -a --remove-destination \"${target}\" \"${link}\"" >> "${rollback}" + + # Atomic replace: build the symlink under a temp name, then rename over the + # regular file. There is no instant where ${link} is absent, so a concurrent + # dlopen either gets the old file or the new symlink — never ENOENT. + ln -sfn "${target}" "${REAL_LIBDIR}/.${link}.tmp.$$" + mv -Tf "${REAL_LIBDIR}/.${link}.tmp.$$" "${REAL_LIBDIR}/${link}" + converted=$(( converted + 1 )) +done + +chmod +x "${rollback}" + +echo +echo "[OK] converted ${converted} file(s) to symlinks." +echo "[OK] rollback script: ${rollback}" + +# --- verify ------------------------------------------------------------------- +echo +echo "=== verification: one inode per SONAME chain ===" +fail=0 +while read -r _hash rest; do + # shellcheck disable=SC2206 + members=( ${rest} ) + inodes="$(for m in "${members[@]}"; do stat -Lc %i "${REAL_LIBDIR}/${m}" 2>/dev/null; done | sort -u | wc -l)" + if [[ "${inodes}" != "1" ]]; then + echo " [FAIL] ${members[*]} -> ${inodes} distinct inodes" + fail=1 + fi +done <<< "${groups}" + +[[ ${fail} -eq 0 ]] && echo " [OK] every normalised chain now resolves to a single inode." +exit "${fail}" diff --git a/ci/rccl_perf_gate/sbatch/lib/workspace.sh b/ci/rccl_perf_gate/sbatch/lib/workspace.sh new file mode 100755 index 000000000..6359d824c --- /dev/null +++ b/ci/rccl_perf_gate/sbatch/lib/workspace.sh @@ -0,0 +1,747 @@ +#!/usr/bin/env bash +############################################################## +# Per-run workspace isolation for the RCCL A/B regression CI. +# +# WHY: the build and detect steps both write to fixed shared paths today — +# builds/{reference,candidate}/ librccl.so for each A/B side +# ab_artifacts/ ab_regression_report.json + rccl_runs.log +# cvs-sbatch/cluster.json regenerated in-tree on every run +# logs/latest global "most recent run" symlink +# With exactly one self-hosted runner those never overlap. The moment a second +# runner exists (see RCCL_CI_REMEDIATION_PLAN.md, issue #1) two PRs share them +# silently — worst case PR A's candidate librccl is measured against PR B's +# reference and the verdict is meaningless but looks legitimate. This module +# gives every run its own workspace so that cannot happen. +# +# DEFAULT-ON: everything here is gated on RCCL_CI_WORKSPACE, which now defaults +# to 1. Set it to 0 for the legacy shared-path behaviour -- every function then +# becomes a no-op that returns success, so this file is safe to source +# unconditionally from the existing scripts. +# +# ws_janitor is the one exception: it reaps regardless of the mode, because the +# rollback switch is exactly when nobody is watching the disk. +# +# LAYOUT (RCCL_CI_WORKSPACE=1): +# runs// +# meta.json provenance: run key, slurm ids, cvs sha, A/B revs, times +# config.json per-run detect config (lib paths + output_dir templated) +# cvs/ detached git worktree of cvs/ at the recorded sha +# cvs-sbatch/ real copy — run.sh regenerates cluster.json in-tree +# builds/ reference/ + candidate/ (see BUILD CACHE below) +# artifacts/ detector output_dir +# logs/ RUN_LOG_DIR +# +# RUN KEY: must be stable across the build job and the detect job, because +# detect loads the librccl that build produced. The Slurm job id differs between +# them, so the key is GITHUB_RUN_ID (+ attempt), falling back to SLURM_JOB_ID for +# manual sbatch and a timestamp for bare local runs. +# +# BUILD CACHE: build_rccl.sh already has a rev-aware skip (lib/.built_rev), so +# naively giving every run a private builds/ would turn every run into a cold +# ~30min LTO build and make the pipeline SLOWER — the opposite of the goal. +# Instead librccl is cached by content under builds/by-rev/-/ and +# the per-run builds//lib is populated from it by hardlink. A given git rev +# built with a given recipe is immutable, so sharing across runs is safe, and the +# merge-base reference is usually identical across PRs targeting develop — so +# most runs hit cache on BOTH sides. Publishing is atomic (build into a private +# tmp dir, then rename), so two runs racing on the same rev cannot corrupt it; +# the loser just adopts the winner's copy. +# Set RCCL_CI_BUILD_CACHE=0 for a fully private cold build per run instead. +############################################################## + +# Deliberately no `set -euo pipefail` here — this file is sourced by callers that +# already set it, and we don't want to impose it on any that don't. + +# Assign only when unset. Callers (run_rccl_build.sh, run_rccl_ab.sh) declare +# this readonly BEFORE sourcing us, and reassigning a readonly variable is a +# fatal error under 'set -e' -- which would break them even with the workspace +# flag OFF, since sourcing happens before any ws_enabled check. +if [[ -z "${RCCL_CI_ROOT:-}" ]]; then + RCCL_CI_ROOT="/it-share/rccl-ci" +fi + +# How many completed run workspaces the janitor keeps, and the age floor below +# which a workspace is never reaped regardless of count (so a run that is still +# in flight, or one someone is actively debugging, survives). +RCCL_CI_WS_KEEP_RUNS="${RCCL_CI_WS_KEEP_RUNS:-20}" +RCCL_CI_WS_KEEP_DAYS="${RCCL_CI_WS_KEEP_DAYS:-14}" + +ws_log() { echo "[workspace $(date +%H:%M:%S)] $*"; } +ws_warn() { echo "[workspace WARN] $*" >&2; } + +# --------------------------------------------------------------------------- +# ws_enabled — the single gate. Every other function short-circuits on this. +# --------------------------------------------------------------------------- +# Default ON as of the workspace rollout. It was introduced opt-in so it could be +# validated against the live pipeline without risking prod; that validation +# passed (build + 4-node detect, 224 sweeps, prod config verifiably untouched), +# and leaving it opt-in would mean the shared-path clobbering returns the moment +# a second runner is added. Set RCCL_CI_WORKSPACE=0 to fall back to the legacy +# shared trees for a one-off debug run. +ws_enabled() { + [[ "${RCCL_CI_WORKSPACE:-1}" == "1" ]] +} + +ws_cache_enabled() { + [[ "${RCCL_CI_BUILD_CACHE:-1}" == "1" ]] +} + +# --------------------------------------------------------------------------- +# ws_run_key — stable identity shared by the build job and the detect job. +# +# GITHUB_RUN_ID is the only id both Slurm jobs of one PR run see, so it is the +# primary key. RUN_ATTEMPT is included because a re-run of a failed workflow +# should get a clean workspace rather than inherit half-written state. +# +# The local-- fallback exists so a hand-run works, but it INVENTS an +# identity: it is only ever correct for a run that is about to create that +# workspace. A read-only query that lands on it is asking "where did this run's +# artifacts go?" while holding no idea which run it is -- see ws_run_key_known. +# --------------------------------------------------------------------------- +ws_run_key() { + if [[ -n "${RCCL_CI_RUN_KEY:-}" ]]; then + echo "${RCCL_CI_RUN_KEY}" + elif [[ -n "${GITHUB_RUN_ID:-}" ]]; then + echo "gh-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT:-1}" + elif [[ -n "${SLURM_JOB_ID:-}" ]]; then + echo "slurm-${SLURM_JOB_ID}" + else + echo "local-$(date +%Y%m%d_%H%M%S)-$$" + fi +} + +# True only when the run key comes from the environment rather than from the +# invented fallback. +ws_run_key_known() { + [[ -n "${RCCL_CI_RUN_KEY:-}" || -n "${GITHUB_RUN_ID:-}" || -n "${SLURM_JOB_ID:-}" ]] +} + +ws_root() { + echo "${RCCL_CI_ROOT}/runs/$(ws_run_key)" +} + +# --------------------------------------------------------------------------- +# ws_recipe_hash — invalidate the build cache when the toolchain changes. +# +# build_rccl.sh stamps only the git rev in .built_rev, so a lib built with an +# older ROCm dist or a different gfx target would look like a cache hit. Fold the +# things that actually change codegen into the cache key. +# +# Everything below is here because it can silently change the emitted code while +# leaving the rev and the dist path identical: +# +# ROCM_DIST resolved through its symlink -- rocm_devel -> 7.14.0a/... +# so a dist bump changes the hash even though the symlink path is +# constant. But the path alone is not enough either: the SDK is +# refreshed IN PLACE (and re-flattens the DMA-BUF symlinks each +# time), so we also fold in a cheap content stamp of the compiler. +# compiler id hipcc's own version string. Two dists can share a directory name +# across a rebuild; they cannot share a clang version banner. +# cmake a cmake upgrade on the build host changes flags and link lines. +# build_rccl.sh the recipe itself. This was the biggest hole: editing the cmake +# arguments in that script -- adding -DCMAKE_BUILD_TYPE, changing +# GPU targets, switching generator -- produced a completely +# different library that the cache happily served as a hit. +# --------------------------------------------------------------------------- +_WS_RECIPE_HASH_CACHE="" +ws_recipe_hash() { + # hipcc --version is a second or so; this is called on every cache lookup and + # store. Memoise per process -- nothing in a single job can change it midway. + if [[ -n "${_WS_RECIPE_HASH_CACHE}" ]]; then + printf '%s' "${_WS_RECIPE_HASH_CACHE}" + return 0 + fi + + local rocm_dist gpu_targets resolved recipe recipe_hash hipcc_id cmake_id p + rocm_dist="${ROCM_DIST:-${RCCL_CI_ROOT}/rocm_devel}" + gpu_targets="${GPU_TARGETS:-gfx950}" + resolved="$(readlink -f "${rocm_dist}" 2>/dev/null || echo "${rocm_dist}")" + + recipe="" + for p in "$(ws_root)/cvs-sbatch/lib/build_rccl.sh" \ + "${RCCL_CI_ROOT}/cvs-sbatch/lib/build_rccl.sh"; do + [[ -f "${p}" ]] && { recipe="${p}"; break; } + done + recipe_hash="norecipe" + if [[ -n "${recipe}" ]]; then + recipe_hash="$(sha256sum "${recipe}" 2>/dev/null | cut -c1-16)" + else + ws_warn "build_rccl.sh not found; recipe changes will NOT invalidate the build cache" + fi + + hipcc_id="$("${resolved}/bin/hipcc" --version 2>/dev/null | head -3 | tr -d '\n')" + [[ -n "${hipcc_id}" ]] || hipcc_id="hipcc-unknown" + cmake_id="$(cmake --version 2>/dev/null | head -1)" + [[ -n "${cmake_id}" ]] || cmake_id="cmake-unknown" + + _WS_RECIPE_HASH_CACHE="$(printf '%s|%s|%s|%s|%s' \ + "${resolved}" "${gpu_targets}" "${recipe_hash}" "${hipcc_id}" "${cmake_id}" \ + | sha256sum | cut -c1-12)" + printf '%s' "${_WS_RECIPE_HASH_CACHE}" +} + +ws_cache_dir_for() { + # $1 = git rev + local rev="$1" + echo "${RCCL_CI_ROOT}/builds/by-rev/${rev}-$(ws_recipe_hash)" +} + +# --------------------------------------------------------------------------- +# ws_init — create the workspace. Idempotent: the build job calls it first, the +# detect job calls it again later and must find the same tree intact. +# --------------------------------------------------------------------------- +ws_init() { + ws_enabled || return 0 + + local ws cvs_src cvs_sha + ws="$(ws_root)" + cvs_src="${RCCL_CI_ROOT}/cvs" + + mkdir -p "${ws}"/{builds,artifacts,logs} || { + ws_warn "could not create workspace at ${ws}" + return 1 + } + + # --- cvs: detached git worktree ------------------------------------------ + # --detach is required: the branch (aimvt-196-rccl-regression-robustness) is + # already checked out in the main worktree and git refuses a second checkout + # of the same branch. Detaching at the resolved sha also gives us exact + # provenance — meta.json records which cvs revision produced the verdict. + if [[ ! -d "${ws}/cvs" ]]; then + local cvs_dirty="" + cvs_sha="$(git -C "${cvs_src}" rev-parse HEAD 2>/dev/null)" + [[ -n "${cvs_sha}" ]] && cvs_dirty="$(git -C "${cvs_src}" status --porcelain 2>/dev/null | head -1)" + + if [[ -z "${cvs_sha}" ]]; then + ws_warn "cvs is not a git checkout; falling back to a plain copy" + cp -a "${cvs_src}" "${ws}/cvs" || return 1 + cvs_sha="unknown" + elif [[ -n "${cvs_dirty}" ]]; then + # A worktree checks out HEAD, so uncommitted changes in cvs/ would be + # SILENTLY DROPPED — the run would execute different code than the tree + # the operator is looking at, and meta.json would record a sha that does + # not describe what ran. Copy instead: correctness beats saving 280M. + ws_warn "cvs has uncommitted changes — using a full copy, not a worktree." + ws_warn " Commit them to get the cheap worktree and exact provenance back." + cp -a "${cvs_src}" "${ws}/cvs" || return 1 + cvs_sha="${cvs_sha}-dirty" + else + # Concurrent worktree adds contend on the repo lock; retry briefly. + local attempt + for attempt in 1 2 3; do + if git -C "${cvs_src}" worktree add --detach "${ws}/cvs" "${cvs_sha}" >/dev/null 2>&1; then + break + fi + [[ ${attempt} -eq 3 ]] && { ws_warn "git worktree add failed after 3 attempts"; return 1; } + sleep $(( attempt * 2 )) + done + fi + else + cvs_sha="$(git -C "${ws}/cvs" rev-parse HEAD 2>/dev/null || echo unknown)" + fi + + # --- cvs-sbatch: real copy ------------------------------------------------ + # Must be a real copy, not a link: run.sh's generate_cluster_config rewrites + # cluster.json in-tree, which is precisely the file two concurrent runs would + # corrupt for each other. + if [[ ! -d "${ws}/cvs-sbatch" ]]; then + cp -a "${RCCL_CI_ROOT}/cvs-sbatch" "${ws}/cvs-sbatch" || return 1 + fi + + ws_write_meta "${cvs_sha}" + + ws_log "workspace ready: ${ws} (cvs ${cvs_sha:0:10})" + return 0 +} + +# --------------------------------------------------------------------------- +# ws_write_meta — provenance. Merged, not overwritten, so the detect job adds +# its Slurm id without erasing the build job's. +# --------------------------------------------------------------------------- +ws_write_meta() { + local cvs_sha="${1:-unknown}" + local ws meta + ws="$(ws_root)" + meta="${ws}/meta.json" + + RCCL_WS_META="${meta}" \ + RCCL_WS_KEY="$(ws_run_key)" \ + RCCL_WS_CVS_SHA="${cvs_sha}" \ + RCCL_WS_RECIPE="$(ws_recipe_hash)" \ + python3 - <<'PY' 2>/dev/null || true +import json, os, datetime + +path = os.environ["RCCL_WS_META"] +try: + with open(path) as fh: + doc = json.load(fh) +except Exception: + doc = {} + +doc.setdefault("run_key", os.environ["RCCL_WS_KEY"]) +doc.setdefault("created_utc", datetime.datetime.utcnow().isoformat() + "Z") +doc["updated_utc"] = datetime.datetime.utcnow().isoformat() + "Z" +doc["cvs_sha"] = os.environ["RCCL_WS_CVS_SHA"] +doc["recipe_hash"] = os.environ["RCCL_WS_RECIPE"] + +for key, env in ( + ("github_run_id", "GITHUB_RUN_ID"), + ("github_run_attempt", "GITHUB_RUN_ATTEMPT"), + ("github_sha", "GITHUB_SHA"), + ("github_pr", "GITHUB_PR_NUMBER"), +): + if os.environ.get(env): + doc[key] = os.environ[env] + +# Slurm job ids accumulate: one for the build job, one for detect. +job = os.environ.get("SLURM_JOB_ID") +if job: + name = os.environ.get("SLURM_JOB_NAME", "job") + jobs = doc.setdefault("slurm_jobs", {}) + jobs[name] = job + +with open(path, "w") as fh: + json.dump(doc, fh, indent=2, sort_keys=True) + fh.write("\n") +PY +} + +# --------------------------------------------------------------------------- +# ws_record — stash an arbitrary key/value in meta.json (revs, verdicts, ...). +# --------------------------------------------------------------------------- +ws_record() { + ws_enabled || return 0 + local key="$1" value="$2" + RCCL_WS_META="$(ws_root)/meta.json" RCCL_WS_K="${key}" RCCL_WS_V="${value}" \ + python3 - <<'PY' 2>/dev/null || true +import json, os +path = os.environ["RCCL_WS_META"] +try: + with open(path) as fh: + doc = json.load(fh) +except Exception: + doc = {} +doc[os.environ["RCCL_WS_K"]] = os.environ["RCCL_WS_V"] +with open(path, "w") as fh: + json.dump(doc, fh, indent=2, sort_keys=True) + fh.write("\n") +PY +} + +# --------------------------------------------------------------------------- +# ws_cache_fetch