ci: run Phoenix GPU benchmarks in one single-node job (backfill + same-hardware)

.github/scripts/run_parallel_benchmarks.sh

@@ -1,9 +1,15 @@
 #!/bin/bash
 # Run PR and master benchmarks and verify outputs.
-# Both SLURM jobs are submitted up front so they run concurrently on
-# compute nodes (fair comparison under the same cluster load), but
-# monitoring happens sequentially to stay within the per-user cgroup
-# memory limit on login nodes (4 GB on Phoenix shared by 7 runners).
+#
+# Phoenix GPU runs both trees in ONE single-node job (see bench-pair.sh): same
+# node, same GPUs, benchmarked back-to-back -- identical hardware with no
+# contention, and it only needs one node so it backfills across a partition list.
+#
+# Other clusters (Frontier) and Phoenix CPU keep the two-job model: both SLURM
+# jobs are submitted up front so they run concurrently on compute nodes (fair
+# comparison under the same cluster load), but monitoring happens sequentially to
+# stay within the per-user cgroup memory limit on login nodes (4 GB on Phoenix
+# shared by 7 runners).
 # Usage: run_parallel_benchmarks.sh <device> <interface> <cluster>
 
 set -euo pipefail
@@ -24,96 +30,119 @@ echo "===================="
 echo "Starting benchmark jobs..."
 echo "===================="
 
-# For Phoenix GPU benchmarks, select a consistent GPU partition so PR and
-# master always land on the same GPU type.
-if [ "$device" = "gpu" ] && [ "$cluster" = "phoenix" ]; then
-    echo "Selecting Phoenix GPU partition for benchmark consistency..."
-    # Require 2 nodes so both jobs can run concurrently on compute.
-    GPU_PARTITION_MIN_NODES=2 source "${SCRIPT_DIR}/select-gpu-partition.sh"
-    BENCH_GPU_PARTITION="$SELECTED_GPU_PARTITION"
-    export BENCH_GPU_PARTITION
-fi
-
-# The bench script must come from the PR tree (master may not have it).
-PR_BENCH_SCRIPT="$(cd "${SCRIPT_DIR}/../workflows/common" && pwd)/bench.sh"
-# Must match the slug computed by submit-slurm-job.sh:
-#   basename("bench.sh") → "bench" → "bench-${device}-${interface}"
+# job_slug names the benchmark YAML each tree writes; bench.yml's bench_diff requires
+# exactly this name, and it is the same whether one job or two produced the results.
+# The SLURM .out/.slurm_job_id basename is NOT always the same string: submit-slurm-job.sh
+# derives that from the submitted script's basename, so the single-node path logs to
+# bench-pair-<device>-<interface> instead. Keep the two apart.
 job_slug="bench-${device}-${interface}"
+log_slug="$job_slug"
 
-# --- Phase 1: Submit both SLURM jobs (no monitoring yet) ---
-echo "Submitting PR benchmark..."
-(cd pr && SUBMIT_ONLY=1 bash "${SCRIPT_DIR}/submit-slurm-job.sh" "$PR_BENCH_SCRIPT" "$device" "$interface" "$cluster")
-pr_job_id=$(cat "pr/${job_slug}.slurm_job_id")
-echo "PR job submitted: $pr_job_id"
-
-echo "Submitting master benchmark..."
-(cd master && SUBMIT_ONLY=1 bash "${SCRIPT_DIR}/submit-slurm-job.sh" "$PR_BENCH_SCRIPT" "$device" "$interface" "$cluster")
-master_job_id=$(cat "master/${job_slug}.slurm_job_id")
-echo "Master job submitted: $master_job_id"
-
-echo "Both SLURM jobs submitted — running concurrently on compute nodes."
-echo "Monitoring sequentially to conserve login node memory."
-
-# --- Phase 2: Monitor sequentially (one at a time on login node) ---
-# On Phoenix 'embers' a long benchmark job can be preempted (PreemptMode=CANCEL,
-# so it is killed rather than requeued). On preemption (run_monitored exit 76)
-# resubmit a fresh job in the same tree and re-monitor, bounded by
-# MAX_PREEMPT_RESUBMITS (the 480m job timeout is the real backstop). Note: a
-# resubmitted job no longer overlaps its counterpart, slightly reducing
-# same-load fairness -- still preferable to failing the run on an infra preempt.
-: "${MAX_PREEMPT_RESUBMITS:=10}"
-monitor_bench_with_resubmit() {  # arg: <dir> (pr|master); sets BENCH_MON_RC
-    local dir="$1"
-    local out="${dir}/${job_slug}.out"
-    local jobid attempt=0 rc
-    jobid=$(cat "${dir}/${job_slug}.slurm_job_id")
-    while :; do
-        rc=0
-        bash "${SCRIPT_DIR}/run_monitored_slurm_job.sh" "$jobid" "$out" || rc=$?
-        if [ "$rc" -ne 76 ]; then
-            BENCH_MON_RC="$rc"
-            return
-        fi
-        if [ "$attempt" -ge "$MAX_PREEMPT_RESUBMITS" ]; then
-            echo "::error::${dir} benchmark preempted ${MAX_PREEMPT_RESUBMITS}x without completing; giving up."
-            BENCH_MON_RC=1
-            return
-        fi
-        attempt=$((attempt + 1))
-        echo "::warning::${dir} benchmark job $jobid was preempted; resubmitting (attempt ${attempt}/${MAX_PREEMPT_RESUBMITS})."
-        rm -f "$out"
-        ( cd "$dir" && SUBMIT_ONLY=1 bash "${SCRIPT_DIR}/submit-slurm-job.sh" "$PR_BENCH_SCRIPT" "$device" "$interface" "$cluster" )
-        jobid=$(cat "${dir}/${job_slug}.slurm_job_id")
-        echo "${dir} benchmark resubmitted as job $jobid"
-    done
-}
-
-echo ""
-echo "=== Monitoring PR job $pr_job_id ==="
-monitor_bench_with_resubmit pr
-pr_exit=$BENCH_MON_RC
-if [ "$pr_exit" -ne 0 ]; then
-    echo "PR job exited with code: $pr_exit"
-    tail -n 50 "pr/${job_slug}.out" 2>/dev/null || echo "  Could not read PR log"
-    # The PR benchmark run genuinely failed (cases crashed/hung/SIGTERM'd, not a
-    # monitor false-positive -- run_monitored_slurm_job.sh re-checks sacct). Fail
-    # the job instead of falling through to the YAML-exists check, which would let
-    # a broken PR pass green as long as a partial YAML was written. Scoped to PR
-    # only: a master/baseline infra flake stays a warning and does not red-cross.
-    exit 1
+if [ "$device" = "gpu" ] && [ "$cluster" = "phoenix" ]; then
+    # --- Phoenix GPU: build + bench BOTH trees in ONE single-node job ---
+    # bench-pair.sh builds master and PR, then benchmarks them back-to-back on
+    # the same node and GPUs: identical hardware, no cross-run contention. Needing
+    # only one node (-G2) lets submit-slurm-job.sh submit to a partition list and
+    # backfill, instead of the old model's two-idle-nodes-in-one-partition wait.
+    # Launched from the PR tree so ../master resolves and mfc.sh load finds a tree.
+    # NOT SUBMIT_ONLY: submit-slurm-job.sh monitors and handles preemption (76)
+    # and node faults (77) itself, so this even gains node-exclude-on-fault, which
+    # the old two-job bench path never had.
+    PAIR_SCRIPT="$(cd "${SCRIPT_DIR}/../workflows/common" && pwd)/bench-pair.sh"
+    log_slug="bench-pair-${device}-${interface}"
+    echo "Phoenix GPU: building and benchmarking master and PR in one single-node job."
+    pair_rc=0
+    ( cd pr && bash "${SCRIPT_DIR}/submit-slurm-job.sh" "$PAIR_SCRIPT" "$device" "$interface" "$cluster" ) || pair_rc=$?
+    if [ "$pair_rc" -ne 0 ]; then
+        # One job builds+benches both trees, so a failure is not attributable to
+        # one side. Fail hard rather than fall through to the YAML-exists check,
+        # which would let a broken run pass green on a stale/partial YAML.
+        echo "::error::single-node bench-pair job failed (exit ${pair_rc})."
+        exit 1
+    fi
+    pr_exit=0
+    master_exit=0
+    echo "Single-node bench-pair job completed successfully."
 else
-    echo "PR job completed successfully"
-fi
+    # --- Other clusters / Phoenix CPU: two concurrent jobs, monitored serially ---
+    # The bench script must come from the PR tree (master may not have it).
+    PR_BENCH_SCRIPT="$(cd "${SCRIPT_DIR}/../workflows/common" && pwd)/bench.sh"
+
+    # Phase 1: Submit both SLURM jobs (no monitoring yet)
+    echo "Submitting PR benchmark..."
+    (cd pr && SUBMIT_ONLY=1 bash "${SCRIPT_DIR}/submit-slurm-job.sh" "$PR_BENCH_SCRIPT" "$device" "$interface" "$cluster")
+    pr_job_id=$(cat "pr/${log_slug}.slurm_job_id")
+    echo "PR job submitted: $pr_job_id"
+
+    echo "Submitting master benchmark..."
+    (cd master && SUBMIT_ONLY=1 bash "${SCRIPT_DIR}/submit-slurm-job.sh" "$PR_BENCH_SCRIPT" "$device" "$interface" "$cluster")
+    master_job_id=$(cat "master/${log_slug}.slurm_job_id")
+    echo "Master job submitted: $master_job_id"
+
+    echo "Both SLURM jobs submitted — running concurrently on compute nodes."
+    echo "Monitoring sequentially to conserve login node memory."
+
+    # Phase 2: Monitor sequentially (one at a time on login node)
+    # On Phoenix 'embers' a long benchmark job can be preempted (PreemptMode=CANCEL,
+    # so it is killed rather than requeued). On preemption (run_monitored exit 76)
+    # resubmit a fresh job in the same tree and re-monitor, bounded by
+    # MAX_PREEMPT_RESUBMITS (the 480m job timeout is the real backstop). Note: a
+    # resubmitted job no longer overlaps its counterpart, slightly reducing
+    # same-load fairness -- still preferable to failing the run on an infra preempt.
+    : "${MAX_PREEMPT_RESUBMITS:=10}"
+    monitor_bench_with_resubmit() {  # arg: <dir> (pr|master); sets BENCH_MON_RC
+        local dir="$1"
+        local out="${dir}/${log_slug}.out"
+        local jobid attempt=0 rc
+        jobid=$(cat "${dir}/${log_slug}.slurm_job_id")
+        while :; do
+            rc=0
+            bash "${SCRIPT_DIR}/run_monitored_slurm_job.sh" "$jobid" "$out" || rc=$?
+            if [ "$rc" -ne 76 ]; then
+                BENCH_MON_RC="$rc"
+                return
+            fi
+            if [ "$attempt" -ge "$MAX_PREEMPT_RESUBMITS" ]; then
+                echo "::error::${dir} benchmark preempted ${MAX_PREEMPT_RESUBMITS}x without completing; giving up."
+                BENCH_MON_RC=1
+                return
+            fi
+            attempt=$((attempt + 1))
+            echo "::warning::${dir} benchmark job $jobid was preempted; resubmitting (attempt ${attempt}/${MAX_PREEMPT_RESUBMITS})."
+            rm -f "$out"
+            ( cd "$dir" && SUBMIT_ONLY=1 bash "${SCRIPT_DIR}/submit-slurm-job.sh" "$PR_BENCH_SCRIPT" "$device" "$interface" "$cluster" )
+            jobid=$(cat "${dir}/${log_slug}.slurm_job_id")
+            echo "${dir} benchmark resubmitted as job $jobid"
+        done
+    }
 
-echo ""
-echo "=== Monitoring master job $master_job_id ==="
-monitor_bench_with_resubmit master
-master_exit=$BENCH_MON_RC
-if [ "$master_exit" -ne 0 ]; then
-    echo "Master job exited with code: $master_exit"
-    tail -n 50 "master/${job_slug}.out" 2>/dev/null || echo "  Could not read master log"
-else
-    echo "Master job completed successfully"
+    echo ""
+    echo "=== Monitoring PR job $pr_job_id ==="
+    monitor_bench_with_resubmit pr
+    pr_exit=$BENCH_MON_RC
+    if [ "$pr_exit" -ne 0 ]; then
+        echo "PR job exited with code: $pr_exit"
+        tail -n 50 "pr/${log_slug}.out" 2>/dev/null || echo "  Could not read PR log"
+        # The PR benchmark run genuinely failed (cases crashed/hung/SIGTERM'd, not a
+        # monitor false-positive -- run_monitored_slurm_job.sh re-checks sacct). Fail
+        # the job instead of falling through to the YAML-exists check, which would let
+        # a broken PR pass green as long as a partial YAML was written. Scoped to PR
+        # only: a master/baseline infra flake stays a warning and does not red-cross.
+        exit 1
+    else
+        echo "PR job completed successfully"
+    fi
+
+    echo ""
+    echo "=== Monitoring master job $master_job_id ==="
+    monitor_bench_with_resubmit master
+    master_exit=$BENCH_MON_RC
+    if [ "$master_exit" -ne 0 ]; then
+        echo "Master job exited with code: $master_exit"
+        tail -n 50 "master/${log_slug}.out" 2>/dev/null || echo "  Could not read master log"
+    else
+        echo "Master job completed successfully"
+    fi
 fi
 
 # --- Phase 3: Verify outputs ---
@@ -146,15 +175,15 @@ if [ ! -f "$pr_yaml" ]; then
     echo "ERROR: PR benchmark output not found: $pr_yaml"
     ls -la pr/ || true
     echo ""
-    tail -n 100 "pr/${job_slug}.out" 2>/dev/null || echo "  Could not read PR log"
+    tail -n 100 "pr/${log_slug}.out" 2>/dev/null || echo "  Could not read PR log"
     exit 1
 fi
 
 if [ ! -f "$master_yaml" ]; then
     echo "ERROR: Master benchmark output not found: $master_yaml"
     ls -la master/ || true
     echo ""
-    tail -n 100 "master/${job_slug}.out" 2>/dev/null || echo "  Could not read master log"
+    tail -n 100 "master/${log_slug}.out" 2>/dev/null || echo "  Could not read master log"
     exit 1
 fi
 

.github/scripts/submit-slurm-job.sh

@@ -126,30 +126,16 @@ elif [ "$device" = "gpu" ]; then
     # Determine GPU partition
     gpu_partition="batch"
     if [ "$gpu_partition_dynamic" = "true" ]; then
-        if [ "$job_type" = "bench" ]; then
-            # Benchmarks compare PR against master, so both jobs must land on the
-            # SAME GPU type or the comparison is meaningless. That rules out a
-            # partition list (SLURM could place PR and master on different
-            # hardware); instead a single partition is picked and pinned across
-            # both jobs via BENCH_GPU_PARTITION. See run_parallel_benchmarks.sh.
-            if [ -n "${BENCH_GPU_PARTITION:-}" ]; then
-                gpu_partition="$BENCH_GPU_PARTITION"
-                echo "Using pre-selected bench partition: $gpu_partition (PR/master consistency)"
-            else
-                source "${SCRIPT_DIR}/select-gpu-partition.sh"
-                gpu_partition="$SELECTED_GPU_PARTITION"
-            fi
-        else
-            # Tests (and build+test) don't compare across hardware, so submit to a
-            # partition LIST and let SLURM start on whichever frees first instead
-            # of pinning one partition and queueing behind it. This restores the
-            # multi-partition backfill that #1299 dropped when it unified test and
-            # bench onto the single-partition bench selector. gpu-l40s (bad
-            # hardware) and gpu-rtx6000 (too slow for the test time limit) are
-            # intentionally omitted.
-            gpu_partition="gpu-h200,gpu-h100,gpu-a100,gpu-v100"
-            echo "Using GPU partition list for test job: $gpu_partition"
-        fi
+        # Submit to a partition LIST and let SLURM start on whichever frees first,
+        # instead of pinning one partition and queueing behind it. Both tests and
+        # benchmarks run on a single node now: benchmarks build and bench BOTH the
+        # master and PR trees in one job on the same GPUs (see bench-pair.sh), so
+        # neither needs the old single-partition bench selector (which required two
+        # idle nodes in the SAME partition at once -- the main bench queue-starver).
+        # gpu-l40s (bad hardware) and gpu-rtx6000 (too slow for the time limit) are
+        # intentionally omitted.
+        gpu_partition="gpu-h200,gpu-h100,gpu-a100,gpu-v100"
+        echo "Using GPU partition list: $gpu_partition"
     fi
 
     case "$cluster" in
@@ -311,8 +297,9 @@ while :; do
         exit 1
     fi
     if [ "$monitor_rc" -eq 77 ]; then
-        # The in-allocation preflight found this node unusable before any real
-        # work started. Exclude it and draw another node.
+        # The in-allocation preflight found this node unusable. Exclude it and draw
+        # another node. Note bench-pair.sh probes only after building both trees, so
+        # a fault there discards those builds and the resubmit repeats them.
         faulted_node=$(bash "$SCRIPT_DIR/node-exclude.sh" node-from "$output_file")
         if [ "$node_attempt" -lt "$MFC_MAX_NODE_RESUBMITS" ]; then
             node_attempt=$((node_attempt + 1))

.github/workflows/bench.yml

@@ -159,8 +159,9 @@ jobs:
       - name: Print Logs
         if:   always()
         run: |
-          cat pr/bench-${{ matrix.device }}-${{ matrix.interface }}.* 2>/dev/null || true
-          cat master/bench-${{ matrix.device }}-${{ matrix.interface }}.* 2>/dev/null || true
+          # bench*- also catches the single-node bench-pair-<d>-<i>.out job log.
+          cat pr/bench*-${{ matrix.device }}-${{ matrix.interface }}.* 2>/dev/null || true
+          cat master/bench*-${{ matrix.device }}-${{ matrix.interface }}.* 2>/dev/null || true
 
       - name: Print Per-Case Logs
         if: always()