[i-kosarev]

Enables the JAX/MaxText workload to run against any RCCL commit rather than only the
librccl the base image ships:

--build-arg RCCL_COMMIT=<sha>          # any commit of ROCm/rocm-systems
--build-arg RCCL_CMAKE_OPTIONS=<flags> # and any build options

Comparing two builds is two images and the same model card twice — no A/B driver, no variant
switch, nothing added to the model path.

Rewritten onto the current mad-rccl. The previous revision targeted the pre-#238 MaxText
harness (the per-model env_scripts/, models.json, and the three jax-maxtext_benchmark_*
scripts), all of which #238 removed. Rebasing was not possible — every file it touched had been
deleted underneath it — so the same capability is re-done on the Primus path: the overlay image,
the median seconds_per_step metric, the skill notes and manifest templates, plus the fixes
found while validating and in review.

Worked example

One pinned RCCL commit, -DFAULT_INJECTION=ON vs =OFF, only librccl.so differing.
MI355X/gfx950, Broadcom Thor2 RoCEv2, base rocm/jax-training:maxtext-v26.6.

Model	Nodes	Inter-node traffic per step	FI=ON	FI=OFF	Delta
8B FP8	1	none	2.9240 s	2.9200 s	+0.14 %
8B FP8	2	grad all-reduce, QPS=1	3.0335 s	3.0320 s	+0.05 %
8B FP8	2	grad all-reduce, QPS=8	3.0590 s	3.0300 s	+0.96 %
8B FP8	4	grad all-reduce, QPS=8	3.2205 s	3.0835 s	+4.44 %
405B FP8	4	FSDP across nodes, QPS=8	61.02 s	17.40 s	+250.8 %

The cost scales with inter-node collective volume. The single-node row is the control — the flag
also compiles a delay into the collective device kernels, so a slowdown under it is not
automatically a network result. Every run is 50 steps (step-time medians only compare at equal
steps), each A/B pair ran back-to-back on the same nodes, and every run confirmed
NET/IB/*/GDRDMA with zero socket fallback. Single repeats.

These percentages are not comparable with the v26.5 numbers this PR previously carried: the
Primus configs have a ~3.5× larger baseline step time, so the same absolute overhead is a much
smaller fraction of it (v26.5 at 4 nodes: 1.064 − 0.888 = 0.176 s; here: 3.2205 − 3.0835 = 0.137 s).

Two settings the table depends on — both are in the shipped templates

• XLA_GPU_AUTOTUNE_LEVEL=0. Primus defaults --xla_gpu_autotune_level=4, and at 24 and 32
  ranks that presents as a dead hang: zero steps, zero NCCL INFO, one thread pinned at 100 % in
  libhsa-runtime64.so.1 for hours. Sixteen ranks survive it. It is compilation, not a defect —
  but it is indistinguishable from a hang without timing it. Note that Primus reads
  XLA_GPU_AUTOTUNE_LEVEL; the MAD-native XLA_AUTOTUNE_LEVEL is a silent no-op on this path, so
  carrying the old key across does nothing. Both templates set it in context.docker_env_vars and
  deployment_config.env_vars, since only the former reaches the container.
• 405B at 4 nodes needs per_device_batch_size=1. The shipped config OOMs at
  241.25 GiB for jit_train_step. Setting dcn_fsdp_parallelism=-1 / ici_fsdp_parallelism=8
  does not change that figure by a byte (the overrides verifiably reach MaxText), so the 241.25 GiB
  is an activation, not parameter or optimizer state. primus_maxtext_llama3.1-405b.template.json
  carries it, so the row is reproducible from a shipped card rather than from overrides typed at
  the prompt.

Traps worth knowing beyond this PR

• The standard SLURM path exports NCCL_IB_DISABLE=1 itself. It is injected into the generated
  sbatch, absent from the manifest, and nothing fails — RCCL reports via NET/Socket and the job
  just runs slower. Worse, an A/B taken over sockets is insensitive. Set it to 0 in both env
  blocks; the templates do.
• --ulimit nofile=1048576 is required at 4+ nodes. A 32-device clique exhausts the default and
  stalls silently at Initialize clique rather than reporting an error.
• A librccl swap must preserve each destination's own RUNPATH — including an empty one. The copy
  in the ROCm pip wheels resolves libamd_smi.so.26 through a RUNPATH reaching a sibling wheel,
  while the candidate is built for /opt/rccl. Dropping the candidate in verbatim leaves a librccl
  that cannot resolve its own dependencies, and import torch dies before step 1. Where the
  destination carried no RUNPATH the candidate's $ORIGIN/../lib has to be removed rather than
  left behind, or that location quietly begins resolving its dependencies somewhere else.
• Gate that swap on the set of unresolved sonames, not on how many there are. Comparing counts
  passes a swap that drops one missing soname and introduces a different one — the number is
  unchanged and the image is broken in exactly the way the gate exists to catch. The overlay records
  the set per target before the swap and fails on any name that was not already there. It stays a
  relative check rather than "must resolve everything", because on this base the pristine
  _rocm_sdk_devel copy already reports six unresolved deps: it is only ever dlopen'd into a
  namespace where those libraries are loaded. Note the limit of any such check — it sees which
  names are unresolved, never which file a name resolved to, which is why the RUNPATH is
  restored rather than merely inspected.

The durable findings are in .claude/skills/mad-slurm-multinode/references/gotchas.md.

Testing

MI355X/gfx950, Broadcom Thor2 RoCEv2. Both images built from scratch and verified in-image: pinned
SHA, .rodata marker 2 vs 0 with a positive control, bnxt_re provider present. 1-, 2- and 4-node
runs go through madengine run end to end. Every run manifest passes the skill's
validate_manifest.sh with zero FAILs; the two templates report the same profile as the existing
ones in assets/manifests/ (their FAILs are the unfilled <FILL_...> placeholders, by design).

The 1-node control could not be run through madengine's single-node SLURM path on this cluster and
needed a node-local workspace: that path opens the manifest by basename while its local-workspace
branch copies it in as build_manifest.json, and with shared_workspace set it then fails to
create run_directory as container-root against NFS root_squash. That is a madengine issue, not
one this PR touches.

[Copilot]

Pull request overview

Enables multi-node execution for JAX/MaxText workloads and adds an RCCL FAULT_INJECTION A/B harness (ROCM-27881) that can pin and validate two RCCL builds differing only by -DFAULT_INJECTION=ON/OFF, including cluster/run tooling to reproduce and attribute the performance delta.

Changes:

• Add a pinned RCCL overlay Dockerfile that builds FI=ON and FI=OFF variants and gates that they differ.
• Add a SLURM slurm_multi self-orchestrating runner to launch one container per node for multi-node MaxText runs.
• Update MaxText run/report scripts to support multi-node topology and add a median steady-state seconds_per_step metric for A/B comparisons.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
scripts/jax-maxtext/run.sh	Adds RCCL variant selection/attestation hooks; wires JAX coordinator env for multi-node; adds 405B model mapping; adjusts performance marker behavior.
scripts/jax-maxtext/run_multinode_rccl.slurm	New SLURM multi-node orchestrator: per-node docker daemon prep, image load, watchdog, and container launch with required env/mounts for A/B.
scripts/jax-maxtext/rccl_verify_mapped.sh	New post-init verification script intended to prove which librccl is actually mapped by a running process.
scripts/jax-maxtext/rccl_variant_select.sh	New selector that installs the requested RCCL variant and logs on-disk verification markers + version banner behavior.
scripts/jax-maxtext/models.json	Adds multi-node-oriented model entries pointing at the RCCL overlay dockerfile prefix.
scripts/jax-maxtext/jax-maxtext_benchmark_setup.sh	Adds tokenizer download behavior for the new 405B model mapping.
scripts/jax-maxtext/jax-maxtext_benchmark_report.sh	Makes topology multi-node-aware; fails loudly on missing env/config; adds MAXTEXT_EXTRA_ARGS; adds 405B dispatch.
scripts/jax-maxtext/jax-maxtext_benchmark_report.py	Drops unused pandas; adds median steady-state step-time parsing/metric and stricter model handling.
scripts/jax-maxtext/install_rdma_core.sh	New rdma-core-from-source installer with a bnxt_re provider gate (Thor2-specific requirement).
scripts/jax-maxtext/env_scripts/qwen3_30b_a3b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/qwen3_14b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/mixtral_8x7b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/llama3.3_70b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/llama3_8b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/llama3_70b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/llama2_7b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/llama2_70b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/gfx950_qwen3_30b_a3b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/gfx950_qwen3_14b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/gfx950_mixtral_8x7b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/gfx950_llama3.3_70b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/gfx950_llama3.1_405b_env.sh	New gfx950 env script for the 405B multi-node config with XLA_AUTOTUNE_LEVEL/XLA_EXTRA_FLAGS hooks.
scripts/jax-maxtext/env_scripts/gfx950_llama3_8b_env.sh	Makes XLA_AUTOTUNE_LEVEL overridable; stops clobbering NCCL_DEBUG; adds XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/gfx950_llama3_70b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/gfx950_llama2_7b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
scripts/jax-maxtext/env_scripts/gfx950_llama2_70b_env.sh	Stops clobbering NCCL_DEBUG; adds append-only XLA_EXTRA_FLAGS hook.
docker/jax_maxtext_rccl_overlay.ubuntu.amd.Dockerfile	New overlay Dockerfile: pinned RCCL commit, dual FI builds with a “must differ” build gate, plus rdma-core 63.0 install option.
benchmark/jax_maxtext/RCCL_FAULT_INJECTION_AB.md	New catalog/recipe page documenting the A/B method, attribution gates, and cluster prerequisites.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

scripts/jax-maxtext/rccl_variant_select.sh:141

• This script notes that exporting NCCL_DEBUG here won’t reach the training process because the selector runs as a child of run.sh, but then it still exports NCCL_DEBUG/NCCL_DEBUG_SUBSYS in attestation mode. That’s misleading (and, in practice, ineffective) and makes it harder to reason about where the attestation env is actually coming from.

if [[ "${RCCL_FI_ATTEST:-0}" == "1" ]]; then
    export NCCL_DEBUG=INFO
    export NCCL_DEBUG_SUBSYS="${NCCL_DEBUG_SUBSYS:-INIT,NET}"
    echo "ATTESTATION MODE: NCCL_DEBUG=INFO NCCL_DEBUG_SUBSYS=$NCCL_DEBUG_SUBSYS (timings from this run are NOT comparable)"
fi

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

scripts/jax-maxtext/run_multinode_rccl.slurm:242

• || true at the end of the docker run | tee pipeline forces the overall command to succeed and (in practice) resets PIPESTATUS, so rc=${PIPESTATUS[0]} becomes 0 even when the container failed. That can mask failures and let the SLURM step look successful.

Remove the trailing || true so the job correctly fails and PIPESTATUS[0] remains meaningful.

  bash -lc "bash ./run.sh --model_repo $MODEL_REPO --quantization '$QUANTIZATION'" 2>&1 | tee -a "$RLOG" || true
rc=${PIPESTATUS[0]}
kill "$WATCHDOG_PID" 2>/dev/null || true
echo "[$HOST rank=$RANK] container exited rc=$rc"
exit "$rc"

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

scripts/jax-maxtext/run.sh:151

• jax-maxtext_benchmark_setup.sh failures (e.g., HF auth / tokenizer download) are not checked here, and run.sh does not run with set -e, so the script can continue into the report step and potentially emit misleading output after a setup failure. Propagate the setup step exit status similarly to the report step.

bash ./jax-maxtext_benchmark_setup.sh -m "$model"

[Copilot]

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

scripts/jax-maxtext/run_multinode_rccl.slurm:69

• BUILDS_DIR defaults to a user-specific path (/home/ilkosare/...), which will fail on most clusters and makes the slurm_multi harness non-portable. Since the script already documents MAD_DOCKER_BUILDS as a required manifest-provided contract, fail fast when it’s missing instead of silently falling back to a personal directory.

BUILDS_DIR="${MAD_DOCKER_BUILDS:-/home/ilkosare/MADstorage/docker_builds}"
IMAGE_TAR="${BUILDS_DIR}/$(echo "$DOCKER_IMAGE_NAME" | tr ':/' '__').tar"

scripts/jax-maxtext/run_multinode_rccl.slurm:263

• JAX_CACHE_HOST also defaults to a user-specific path (/home/ilkosare/...). This makes the run harness non-portable and can fail in environments where that path doesn’t exist. Prefer a generic default (e.g. under the submit dir) so the script works out of the box, while still allowing the manifest/env to override it.

export JAX_CACHE_HOST="${JAX_CACHE_HOST:-/home/ilkosare/MADstorage/ilk-jaxmt/cache/jax}"
mkdir -p "$JAX_CACHE_HOST"; chmod -R o+rwX "$JAX_CACHE_HOST" 2>/dev/null || true

scripts/jax-maxtext/rccl_variant_select.sh:143

• The comments here state that nothing is exported from this selector because it runs as a child of run.sh, but the attestation block still exports NCCL_DEBUG/NCCL_DEBUG_SUBSYS. Those exports cannot affect the training process (child env doesn’t propagate), so the block is misleading and contradicts the stated design. Either drop these exports or rewrite the comment to clarify they only affect the selector process itself.

# ----------------------------------------------------- verification 2 of 4 ---
# The RCCL version banner. NOTE: this script runs as a CHILD of run.sh, so exporting
# NCCL_DEBUG here would die with it - the env scripts set `NCCL_DEBUG="${NCCL_DEBUG:-VERSION}"`
# instead, which is what actually puts the banner in the log. Nothing is exported here on
# purpose; the caller is responsible for asserting the banner is present afterwards.

# Verification 3 of 4: the mapped-library check, which can only run once the
# training process exists - see rccl_verify_mapped.sh, invoked after JAX initialises.
#
# Verification 4 of 4: the shim announces itself at runtime with
#   NET/IB ops-fault: installed shims on context 0x...
# (INFO, NCCL_NET subsystem) once per ibv context. That is the strongest possible
# evidence - it proves the shim actually installed, not merely that it was compiled
# in. It requires NCCL_DEBUG=INFO + NCCL_DEBUG_SUBSYS covering NET, which is verbose
# enough to perturb step times, so it belongs in a SEPARATE short attestation run
# per variant, NOT in the timed A/B runs. Measuring and attesting in the same run
# would mean reporting numbers taken under logging overhead.
if [[ "${RCCL_FI_ATTEST:-0}" == "1" ]]; then
    export NCCL_DEBUG=INFO
    export NCCL_DEBUG_SUBSYS="${NCCL_DEBUG_SUBSYS:-INIT,NET}"
    echo "ATTESTATION MODE: NCCL_DEBUG=INFO NCCL_DEBUG_SUBSYS=$NCCL_DEBUG_SUBSYS (timings from this run are NOT comparable)"
fi

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Suppressed comments (1)

scripts/jax-maxtext/run_multinode_rccl.slurm:252

• JAX_CACHE_HOST defaults to a hard-coded user-specific path (/home/ilkosare/...). Since this directory is mounted into every container, a non-portable default risks either failing or writing caches into an unexpected location on other clusters. Consider defaulting relative to the submit/output directory (or requiring it explicitly) to keep the script self-contained.

export JAX_CACHE_HOST="${JAX_CACHE_HOST:-/home/ilkosare/MADstorage/ilk-jaxmt/cache/jax}"
mkdir -p "$JAX_CACHE_HOST"; chmod -R o+rwX "$JAX_CACHE_HOST" 2>/dev/null || true

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Suppressed comments (1)

scripts/jax-maxtext/run_multinode_rccl.slurm:67

• BUILDS_DIR defaults to a user-specific absolute path (/home/ilkosare/...). That makes the script non-portable and can lead to confusing failures if MAD_DOCKER_BUILDS isn’t set. Prefer requiring MAD_DOCKER_BUILDS explicitly (it’s already part of the stated env contract) and derive IMAGE_TAR from it.

  esac
done
BUILDS_DIR="${MAD_DOCKER_BUILDS:-/home/ilkosare/MADstorage/docker_builds}"
IMAGE_TAR="${BUILDS_DIR}/$(echo "$DOCKER_IMAGE_NAME" | tr ':/' '__').tar"

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

.claude/skills/mad-slurm-multinode/assets/manifests/jax_maxtext_llama-3.1-8b.template.json:81

• This reusable template hard-codes a Thor2-only scheduler workaround. On CX7/AINIC or any SLURM partition that uses GPU GRES, suppressing --gpus-per-node can under-request GPUs or leave them unavailable to the job. Default this to the normal directive and let Thor2 users opt out, as documented in references/cluster-types.md:77-83.

      "_comment_skip_gpus": "This cluster's SLURM rejects the generated --gpus-per-node directive; without this the job is refused before launch.",
      "skip_gpus_directive": true,

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.claude/skills/mad-slurm-multinode/assets/manifests/jax_maxtext_llama-3.1-8b.template.json:64

• The reusable template cannot select the advertised FAULT_INJECTION A/B arm. Although the image tag asks for <FILL_BUILD_VARIANT>, docker_build_arg omits RCCL_CMAKE_OPTIONS, so a fully filled template always builds the commit's default and can label that build as ON or OFF incorrectly. Add the same explicit build argument used by example_manifest_multinode_rccl.json.

      "RDMA_CORE_VERSION": "<FILL_RDMA_CORE  empty to keep the base image rdma-core; 63.0 is the Broadcom-Thor2 bnxt_re EFAULT fix, see references/cluster-types.md>"

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.claude/skills/mad-slurm-multinode/assets/manifests/jax_maxtext_llama-3.1-8b.template.json:5

• BUILD_GPU_TARGETS below explicitly allows gfx942, but this cache key always says gfx950. A gfx942 build can consequently reuse an existing gfx950 image/tar with the same commit and variant instead of rebuilding for the requested architecture. Include the target placeholder in the tag so architecture-specific artifacts cannot collide.

      "docker_image": "jax-maxtext-rccl:<FILL_RCCL_COMMIT>-<FILL_BUILD_VARIANT>-gfx950",

.claude/skills/mad-slurm-multinode/assets/manifests/jax_maxtext_llama-3.1-8b.template.json:65

• The template names a <FILL_BUILD_VARIANT> in the image tag but never passes that variant to the Docker build. Filling this template for FI=ON and FI=OFF therefore produces the Dockerfile's same default build under two tags, so it cannot perform the advertised A/B comparison. Add RCCL_CMAKE_OPTIONS here as a required placeholder, as the worked example does.

    "docker_build_arg": {
      "BASE_DOCKER": "rocm/jax-training:maxtext-v26.5",
      "RCCL_COMMIT": "<FILL_RCCL_COMMIT>",
      "BUILD_GPU_TARGETS": "<FILL_GPU_TARGETS  gfx942=MI300X/MI325X, gfx950=MI350X/MI355X>",
      "RDMA_CORE_VERSION": "<FILL_RDMA_CORE  empty to keep the base image rdma-core; 63.0 is the Broadcom-Thor2 bnxt_re EFAULT fix, see references/cluster-types.md>"

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

scripts/jax-maxtext/run.sh:93

• An explicitly supplied JAX_COORDINATOR_PORT is overwritten with 29500 whenever MASTER_PORT is absent. Existing standalone launchers use custom JAX ports, so routing them through this wrapper would make ranks connect to the wrong endpoint. Retain the native JAX value as the fallback.

  export JAX_COORDINATOR_PORT="${MASTER_PORT:-29500}"

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[Copilot]

🟡 Changes recommended

Multi-node rendezvous setup, RCCL build arguments, metric gating, and the advertised 405B fix are incomplete.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 5
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The 405B template retains a known hanging autotune default, and empty destination RUNPATHs are not preserved during library replacement.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

.claude/skills/mad-slurm-multinode/references/gotchas.md:227

• This says level 4 was never timed at 32 ranks and cites only the retired harness, but the PR's current Primus validation reports that the default level 4 stalled for hours at both 24 and 32 ranks. As written, the durable guidance omits the observed failure and tells users to repeat it; update this section to distinguish the current Primus result and document level 0 for these RCCL A/B runs.

• Files reviewed: 6/6 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The broad library replacement and optional rdma-core surgery require final human validation despite no specific defect found.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[mkuznet1]

LGTM