feat(rccl): enable multi-node MADEngine CI tests [AICOMRCCL-1332] - #10396
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
There was a problem hiding this comment.
Pull request overview
This PR expands RCCL CI coverage by adding MADEngine workload execution (including multi-node) to the existing TheRock-driven CI pipeline, alongside shared helper utilities and test runners.
Changes:
- Adds a new MADEngine test workflow and wires it into the scheduled RCCL CI on gfx950 (Ruby SLURM runner).
- Introduces a MADEngine workload runner (
test_madengine.py) that builds an RCCL overlay image, generates a manifest, runs MADEngine, and parses/records performance metrics. - Adds/duplicates JAX and PyTorch distributed smoke test runners and shared CI utilities under
projects/rccl/ci/scripts.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/rccl/ci/scripts/test_pytorch_c10d.py | PyTorch c10d NCCL test runner for CI-built RCCL with cloning + JUnit parsing + reporting. |
| projects/rccl/ci/scripts/test_madengine.py | MADEngine workload runner: overlay image build, manifest generation, execution, metrics parsing, regression checks, reporting. |
| projects/rccl/ci/scripts/test_jax_collective.py | JAX collective smoke test runner for CI-built RCCL with JUnit parsing + reporting. |
| projects/rccl/ci/scripts/rccl_ci_utils.py | Shared helpers for artifact discovery, JUnit parsing, GitHub summary/output, and notifications. |
| .github/workflows/therock-rccl-test-madengine.yml | New reusable workflow to run MADEngine workloads on Ruby SLURM runner and upload artifacts. |
| .github/workflows/therock-rccl-ci.yml | Changes scheduled cadence to nightly; adds madengine_nodes input and passes it through. |
| .github/workflows/therock-rccl-ci-linux.yml | Adds madengine_nodes input and a scheduled/dispatch MADEngine test job for gfx950. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
e4af659 → c3a9345 → 4694545 → 0af47b2 |
|
#163 (merged in madengine - fixes for ruby) - You can use this to reduce the overall delta (by reducing the number of patches in MADEngine that are currently in [feat(rccl): enable multi-node MADEngine CI tests AICOMRCCL-1332 by prasanna-amd · Pull Request #1…](#10396)) |
i-kosarev
left a comment
There was a problem hiding this comment.
My main and pretty much only concern is what was commented in #9055 about build-time gate that confirms the copied .so's hash/symbol matches the CI-built artifact: #9055 (comment)
|
@i-kosarev This is now addressed in commit aa2cdf8. The script extracts an md5 + version string ( Verified in job 27169 — full output: Could you re-review when you get a chance? |
Thanks — the fingerprint mechanism itself looks right, and job 27169 shows it if args.skip_overlay_build and rccl_fingerprint.get("md5"):
rccl_ok, rccl_msg = verify_rccl_replacement(...)and
|
|
@i-kosarev Addressed all three points in 60a1f51:
Tested against job 27169 (2N run): Could you re-review? |
There was a problem hiding this comment.
I am good with the code, but we need actual GH run passing. Right now it has issues: https://github.com/ROCm/rocm-systems/actions/runs/32419399327
9e03f79 to
eba4ea4
Compare
Remove --registry, docker login step, and packages:write permission. Images reach compute nodes via MAD_DOCKER_BUILDS — the registry push is unnecessary and the elevated permission causes startup_failure when called from therock-ci.yml which only grants contents:read.
The caller in therock-rccl-ci-linux.yml was still requesting packages:write for the madengine job, but the parent workflow only grants contents:read — causing startup_failure. Drop it from both the caller and callee since images reach nodes via MAD_DOCKER_BUILDS.
- Update TheRock checkout to a30607ad (2026-08-24) - Lift arch-specific env vars to workflow-level, replace case block with a simple arch guard - Split pip install into its own step for clearer error attribution
25c31d6 to
09f7072
Compare
pip3 is not in PATH on ruby-linux-slurm-scale-runner nodes.
|
Will merge given code owner approval. The TheRock CI failures are only known infra issues, unrelated to the MADEngine CI-test change:
|
[AICOMRCCL-1332] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add Llama-3.1-70B training workload (via MADEngine) to the RCCL CI pipeline on the Ruby cluster. The mechanism is a Docker overlay image that swaps the CI-built librccl.so into a rocm/primus base container, enabling end-to-end validation of RCCL builds against real training workloads without rebuilding the full PyTorch/ROCm stack. Key components: - test_madengine.py: orchestration script — overlay build, manifest generation, SLURM dispatch, live log metric extraction, JSONL perf datastore with per-precision rolling regression detection - therock-rccl-test-madengine.yml: workflow for MADEngine workloads - CI schedule changed from weekly to nightly (06:17 UTC) - CI scripts moved to projects/rccl/ci/scripts/ (from .github/scripts/) Clones pinned: madengine at ec4de0b58c49, MAD at 688828bd9d4a on the mad-rccl branch. HF token passed via process environment only (not written to manifest artifact). Validated single-node (8x MI325X): BF16: 773.9 TFLOP/s/GPU, 1722 tokens/s/GPU (50/50 iterations) FP8: 1249.4 TFLOP/s/GPU, 2780 tokens/s/GPU (50/50 iterations) [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
runs Fixes discovered during manual validation on Ruby cluster: - Add --network=none to Docker overlay build (bridge not available on all compute nodes) - Pin SLURM job to the overlay build node when no registry is configured (image only exists locally) - Disable madengine node health check that overrides nodelist - Strip multi-NIC config for single-node runs (Gloo requires all listed NICs to exist, unlike NCCL) - Fix MAD scripts path (primus_megatron-lm, not primus/megatron-lm) - Use full SHA for MAD pin - Smart MAD clone: verify HEAD before fetch+checkout - Handle PermissionError when saving run artifacts to shared dir Validated: 1N/8GPU llama-3.1-70b-training on Ruby BF16: 763.2 TFLOP/s/GPU, 1698.4 tok/s/GPU FP8: 1254.7 TFLOP/s/GPU, 2792.2 tok/s/GPU [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
=?UTF-8?q?=E2=80=94=20manifest,=20provenance,=20results?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix critical issues from mkuznet1 and i-kosarev review: Manifest structure (generate_manifest): - Move slurm config into deployment_config.slurm (was root-level, ignored by madengine) - Move env vars into context.docker_env_vars and deployment_config.env_vars (root-level env was dead) - Move container mounts into context.docker_mounts (root-level container_mounts was dead) - Add multiple_results to built_models card so madengine can produce per-precision structured output (root cause of empty metrics) - Set n_gpus to -1 (madengine resolves per-node) - Leave training_precision empty (card runs both BF16 and FP8) - Add GLOO_SOCKET_IFNAME to docker_env_vars - Remove MAD_MULTI_NODE_RUNNER (noise for primus launcher) - Add qos to deployment_config.slurm (was silently dropped) - Add docker_run_options from reference template Simplify run_madengine: - Remove redundant additional_context (slurm, distributed, env_vars) since manifest now has deployment_config in the right place and madengine merges it automatically RCCL commit provenance (get_rccl_commit): - Remove git rev-parse HEAD fallback — in CI this returns TheRock's pinned commit (constant), causing stale image cache hits - Use RCCL_COMMIT_HASH env, then GITHUB_RUN_ID, then sha256 of librccl.so for unique image tags Push tag format (build_rccl_overlay_image): - Replace both / and : in base image name to avoid invalid Docker reference with two colons Results parsing: - Replace parse_perf_csv with parse_perf_results reading perf_entry_super.json (31 fixed columns, per-precision rows) - Fall back to perf.csv with all rows (was collapsing to last row) - Use per-run status in datastore (was stamping one status on all) - Copy perf_entry_super files to run artifacts - Require metric_value for exit code override (was only checking iteration count) Workflow YAML: - Add secrets: inherit to therock-test-madengine job (HF_TOKEN was silently empty) - Add packages: write permission (needed for GHCR push) - Upload perf_entry_super files as CI artifacts [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
MADEngine metrics Drive regression checks and datastore writes from structured output (perf_entry_super.json) instead of live-log scraping. Addresses three review items from mkuznet1 (Aug 12): - Filter structured rows by metric_key and key by precision instead of overwriting metric_value on every row (last-row-wins bug) - Invert priority: structured results are primary for regression and datastore, live-log scraping is fallback only when structured data is missing - Build a single precision_results list consumed by all downstream stages (exit-code override, regression, datastore, report) Also fixes: - Preserve header-only runs (precision detected, zero iterations) as incomplete instead of silently discarding them - Record status=fail with metric_value=None when no results exist (was recording pass with no metric) [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
- Only pin SLURM job to build node for single-node runs; multi-node needs SLURM to allocate across nodes freely - Remove --network=none from docker build so overlay push to GHCR works - Drop vip_prio QoS (DenyQos on meta64 partition rejects it) [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
=?UTF-8?q?=5FSOCKET=5FIFNAME=20=E2=80=94=20compute=20nodes=20only=20have?= =?UTF-8?q?=20fenic0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gloo fails with "Unable to find address for: enp49s0f0np0" because that interface doesn't exist on Ruby compute nodes. Only fenic0 is present across all nodes. [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
The container's libibverbs/bnxt_re provider doesn't match the host kernel's RDMA driver, causing ibv_create_qp to fail with "Bad address" on multi-node runs. Mount the host's /usr/lib64/libibverbs* into /host_ib_libs/ and set LD_LIBRARY_PATH + LIBIBVERBS_DRIVER_PATH so RCCL uses the host-matching verbs providers. Controlled by the existing mount_host_ib_libs flag in CLUSTER_CONFIGS. [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
docker_mounts convention is {container_path: host_path}, not the
reverse. The previous commit had the mapping backwards, mounting
empty container paths onto the host instead of host libs into the
container.
[rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
mount Container ships rdma-core 50 (rdmav34 ABI) but Ruby hosts run rdma-core 61 (rdmav59). The ABI mismatch causes ibv_create_qp to fail with "Bad address" on multi-node runs. Mount the host's libibverbs.so, provider directory, and libibumad.so over the container's symlink targets so the existing symlinks resolve to the host's rdma-core 61 binaries. [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
The host's libibverbs.so has /usr/lib64/libibverbs/ hardcoded as its provider search path. Previous mounts placed providers at the container's Ubuntu path (/usr/lib/x86_64-linux-gnu/libibverbs/) which the host binary never searches, causing "cannot open shared object" for every rdmav59 provider. Mount the provider directory at /usr/lib64/libibverbs/ and replace the container's libibverbs.so.1 with the host's via the standard Ubuntu library path. Also use soname symlinks instead of version-specific filenames for portability. [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
correctly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit madengine emits metric names like "tok_per_s_per_gpu" and "TFLOPS_per_gpu" in perf_entry_super.json, but the parser compared against our config key "tokens_per_second_per_gpu" — every row was skipped, leaving precision_results empty and status=fail. Also: training_precision is empty in the JSON; the actual value lives in multi_results.precision. And madengine writes "SUCCESS" not "PASS". Fixes: - Map config metric_key to madengine aliases via _METRIC_ALIASES - Fall back to multi_results.precision when training_precision empty - Accept "SUCCESS" (case-insensitive) as a passing status - Capture TFLOPS from companion rows and attach to precision_results [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
When --skip-overlay-build is used, the container's bundled RCCL was used instead of the CI artifact. Now the artifact librccl.so (and kpack files) are bind-mounted over the container's SDK library paths so we actually test the intended RCCL build. [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
Extract version and md5 from the CI artifact pre-flight, then verify post-run that the container's RCCL version string matches. Catches cases where the container's bundled RCCL is used instead of the bind-mounted CI artifact. Works across all frameworks (JAX, Megatron, SGLang, vLLM) since RCCL always prints its version during init. [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
non-overridable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback: 1. Remove --skip-overlay-build gate so verification runs in both overlay and bind-mount modes (the nightly CI path builds an overlay). 2. Check every *node_*.out log, not just node_0 — a library that reached only one node still fails. 3. Track rccl_verification_failed separately so the exit-code override block cannot clear a verification failure. [rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
[rocm-systems] ROCm/rocm-systems#10396 (commit 2b51bf8)
TheRock Submodule Bump ActivityNewest first
|
Summary
NCCL_SOCKET_IFNAME, Docker--network=noneperf_entry_super.jsonmetric name parsing so results report correctly (tok/s/GPU, TFLOPS/GPU)JIRA ID : AICOMRCCL-1332
Validation (Ruby cluster, job 26928)
Key changes
test_madengine.py: multi-node manifest generation, host IB lib bind-mounts, metric parsing fixestherock-rccl-test-madengine.yml: new workflow for MADEngine workloadsrccl_ci_utils.py: shared CI utilities (GitHub summary, outputs)test_jax_collective.py,test_pytorch_c10d.py: additional CI test scriptsTest plan
perf_entry_super.jsonparsing produces correct metricspython3 -m py_compilepasses on all new scripts🤖 Generated with Claude Code