Update SLURM skills to handle launching from the login nodes

[haok1402]

No description provided.

[haok1402]

@claude review

[claude]

Performance Review

No performance concerns. This PR only touches launch orchestration (deriving the torchrun rendezvous host from SLURM_STEP_NODELIST via scontrol) and SLURM skill docs. The scontrol show hostnames call runs once at process launch, not on any per-step/per-layer/per-micro-batch path, so it can't affect training step time or peak memory. No hot-path code, kernels, collectives, comm overlap, or compiled regions are changed.

[claude]

Compactness Review

No compactness issues. This PR is skill documentation plus small shell tweaks (deriving the torchrun rendezvous host from the SLURM nodelist, expanded launch-with-slurm guidance); it adds no wrappers, config knobs, or defensive dependency/GPU guards. The RDZV_HOST line repeated across the seven launch scripts is intentional per-script self-containment, not extractable duplication, and the double localhost fallback guards two distinct failures rather than being redundant.

[claude]

Consistency: The new comment says "anchor the step with the launch-with-slurm skill," but the command has no --jobid/--overlap. Per that skill, --jobid is required when $SLURM_JOB_ID is unset (the login-node case this PR is enabling) and --overlap is needed alongside a running step — so as written this snippet does not anchor. Either show the anchored form (srun --jobid=<jobid> --overlap -W 0 -o …) or note the command assumes you're already inside the allocation.

[claude]

Consistency Review

The rendezvous-host change (SLURM_LAUNCH_NODE_IPADDR → scontrol show hostnames "$SLURM_STEP_NODELIST") is applied uniformly across every launcher (examples/pretrain_lm/launch.sh, all benchmark .sh, capture/validate scripts) with no stale prose left describing the old ipaddr path — grep for SLURM_LAUNCH_NODE_IPADDR and the old $SLURM_JOB_ID-gated wording is clean. No stale references fall outside the diff.

Two consistency issues, both inline:

• launch-with-slurm -n guidance says "match -N × GPUs" for torchrun-style runs — that over-spawns; torchrun needs one srun task per node.
• setup-benchmark-inputs added an "anchor the step with the launch-with-slurm skill" comment next to an srun command that carries neither --jobid nor --overlap, so it does not actually anchor from a login node (the case this PR enables).

[claude]

Correctness Review

Scope is SLURM launch plumbing: the rendezvous host now derives from scontrol show hostnames "${SLURM_STEP_NODELIST:-localhost}" | head -1 || echo localhost instead of ${SLURM_LAUNCH_NODE_IPADDR:-localhost}. The endpoint-derivation logic is sound (all step tasks compute the same first hostname, so c10d rendezvous lands on a participating node), and the three scripts with set -o pipefail (examples/pretrain_lm/launch.sh, launch_validate.sh, launch_capture.sh) fall back to localhost correctly when scontrol is missing.

One correctness issue: the 7 benchmarks/pretraining/**/*.sh scripts have no set -o pipefail, so the pipeline's exit status is head's (always 0) and the || echo localhost fallback is dead code. On a bare non-SLURM host — exactly the single-node (h100-1n8g) benchmark target — scontrol errors, head reads EOF and exits 0, and RDZV_HOST is left empty, yielding --rdzv-endpoint=:15213 and a broken rendezvous. This regresses the prior ${SLURM_LAUNCH_NODE_IPADDR:-localhost} behavior. Inline threads on each of the 7 files carry a pipefail-independent drop-in fix.

No test/loss evidence is needed here — this is launch-only plumbing with no effect on numerics, gradients, or parallelism math.

[haok1402]

@claude review pipefail and -n option ; I've fixed both in the latest two commits.

[claude]

Performance Review

No performance concerns. This PR is SLURM launch orchestration (deriving the torchrun rendezvous host via scontrol show hostnames "$SLURM_STEP_NODELIST") plus skill docs. The scontrol call runs once at process launch, off any per-step/per-layer/per-micro-batch path, so it can't affect step time or peak memory. No hot-path code, kernels, collectives, comm overlap, or compiled regions are touched.

[claude]

Compactness Review

No compactness issues. The change is flat — skill-doc prose updates plus a repeated RDZV_HOST=$(scontrol show hostnames …) derivation across seven launcher scripts. It adds no wrappers, factories, registries, or unused config; the duplicated rendezvous-host one-liner is consistent with the local-editability convention (each launcher is self-contained), so it should stay duplicated rather than be hoisted. The ${SLURM_STEP_NODELIST:-localhost} default and || echo localhost fallback cover the real non-SLURM path, not a dead condition.

[claude]

Correctness Review

No correctness issues. The two previously-flagged items are resolved: all 7 benchmarks/pretraining/**/*.sh scripts now carry set -euo pipefail, and with pipefail on the scontrol show hostnames "${SLURM_STEP_NODELIST:-localhost}" | head -1 || echo localhost idiom correctly falls back to localhost when scontrol is missing/fails (the pipeline returns non-zero, || echo localhost recovers the substitution, so RDZV_HOST is never left empty and set -e doesn't trip) — the non-SLURM single-node (h100-1n8g) target rendezvouses correctly again. The rendezvous-host derivation is sound across ranks (every step task computes the same first hostname of the allocation, always a participating node, unlike the old launch-node ipaddr when launching from a login node), and no stale SLURM_LAUNCH_NODE_IPADDR references remain. Launch-only plumbing with no effect on numerics, gradients, or parallelism math, so no loss/metric evidence is required.

[claude]

Consistency Review

The PR rewrites launch-with-slurm to drop the $SLURM_JOB_ID-set gate and support login-node launches via --jobid/--overlap/--gres, and swaps the torchrun rendezvous host from SLURM_LAUNCH_NODE_IPADDR to scontrol show hostnames "$SLURM_STEP_NODELIST" across every launcher. That cleanup is consistent: no lingering SLURM_LAUNCH_NODE_IPADDR, no stale $SLURM_JOB_ID-gating prose in sibling skills, and all example/benchmark launchers now derive the rdzv host the same way. docs/user-guide.md and AGENTS.md describe the launchers generically, so nothing there went stale — no out-of-diff stale reference found.

One inline nit: the setup-benchmark-inputs example gained an "anchor the step" comment but the srun command still lacks the --jobid that "anchor" refers to.