Merge branch 'master' into cont_damage_fix

Author: sbryngelson

.github/scripts/check_coverage_map_health.py

@@ -4,7 +4,7 @@
 from pathlib import Path
 
 sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "toolchain"))
-from mfc.test.coverage import COVERAGE_MAP_PATH, _git, load_map, map_health  # noqa: E402
+from mfc.test.coverage import COVERAGE_MAP_PATH, load_map, map_health, run_git  # noqa: E402
 from mfc.test.cases import list_cases  # noqa: E402  (returns the current test list)
 
 MAX_AGE_DAYS = 10
@@ -39,7 +39,7 @@ def verified_sha(cwd=None):
     caller must read that as undeterminable and fall back to the wall-clock age rule, not
     as a failure -- an absent ref is not evidence of a broken refresh.
     """
-    rev = _git(["rev-parse", "--verify", "--quiet", f"{VERIFIED_REF}^{{commit}}"], cwd)
+    rev = run_git(["rev-parse", "--verify", "--quiet", f"{VERIFIED_REF}^{{commit}}"], cwd)
     return rev.stdout.strip() or None
 
 
@@ -52,10 +52,10 @@ def verified_after_last_change(git_sha, cwd=None):
     """
     if not git_sha:
         return None
-    last = _git(["log", "-1", "--format=%H", "--", *COVERAGE_RELEVANT_PATHS], cwd)
+    last = run_git(["log", "-1", "--format=%H", "--", *COVERAGE_RELEVANT_PATHS], cwd)
     if last.returncode != 0 or not last.stdout.strip():
         return None  # shallow clone or no such commit -> fall back to the age rule
-    ancestor = _git(["merge-base", "--is-ancestor", last.stdout.strip(), git_sha], cwd)
+    ancestor = run_git(["merge-base", "--is-ancestor", last.stdout.strip(), git_sha], cwd)
     return {0: True, 1: False}.get(ancestor.returncode)  # anything else -> None (unknown sha, shallow history)
 
 

.github/scripts/ci-outage.sh

@@ -35,6 +35,15 @@ output_file="$2"
 echo "Submitted batch job $job_id"
 echo "Monitoring output file: $output_file"
 
+# Put the one thing a reader needs on the run's summary page. Without this,
+# learning why a job failed means opening a log of tens of thousands of lines --
+# and an infrastructure fault looks exactly like a test failure until you do.
+# Silent when not running under Actions.
+ci_summary() {
+  [ -n "${GITHUB_STEP_SUMMARY:-}" ] || return 0
+  printf '%b\n' "$1" >> "$GITHUB_STEP_SUMMARY"
+}
+
 # Robustly check SLURM job state using squeue with sacct fallback.
 # Returns the state string (PENDING, RUNNING, COMPLETED, FAILED, etc.)
 # or "UNKNOWN" if both commands fail.
@@ -213,8 +222,14 @@ while true; do
   sleep "$MFC_MONITOR_POLL_SECONDS"
 done
 
-# Give tail a moment to flush the final lines, then stop streaming.
+# Give tail a moment to flush the final lines, then stop streaming. Whether it
+# was still alive decides how much needs reprinting below: if it streamed the
+# whole job, printing the file again just doubles every log.
 sleep 2
+streamed_ok=0
+if kill -0 "${tail_pid}" 2>/dev/null; then
+  streamed_ok=1
+fi
 kill "${tail_pid}" 2>/dev/null || true
 tail_pid=""
 
@@ -238,9 +253,20 @@ if [ -f "$output_file" ]; then
   done
 fi
 
+# Reprint only what streaming may have missed. `tail -f` above already emitted
+# the whole file as it was written, so cat'ing it again duplicated every job's
+# output -- measured at 3 copies of each line on a GPU job, and 65,000 lines of
+# offload diagnostics repeated for a single fault. The reprint exists solely as
+# a safety net for a tail that died mid-job, so it is bounded when tail survived
+# and complete only when it did not.
 echo ""
-echo "=== Final output ==="
-cat "$output_file"
+if [ "${streamed_ok:-0}" -eq 1 ]; then
+  echo "=== Final output (tail; the full log streamed above) ==="
+  tail -n "${MFC_MONITOR_FINAL_LINES:-40}" "$output_file"
+else
+  echo "=== Final output (streaming stopped early; reprinting in full) ==="
+  cat "$output_file"
+fi
 
 # Check exit status with sacct fallback
 exit_code=""
@@ -267,26 +293,32 @@ if [ -z "$exit_code" ]; then
   exit 1
 fi
 
-# Infrastructure verdicts from the in-allocation preflight come back as the
-# job's own exit code. Relay them verbatim: flattening them to 1 would leave the
-# submit wrapper unable to tell "this node is unusable" (exclude it and try
-# again) from "the tests failed" (report it).
+# The preflight's node-fault verdict comes back as the job's own exit code.
+# Relay it verbatim: flattening it to 1 would leave the submit wrapper unable to
+# tell "this node is unusable" (exclude it and try again) from "the tests
+# failed" (report it).
+faulted_node=$(grep -oE 'MFC_FAULT_NODE=[^ ]+' "$output_file" 2>/dev/null | tail -n1 | cut -d= -f2 || true)
+
 case "$exit_code" in
   77:*)
     echo "Job $job_id failed preflight: the node is unusable — signaling caller to exclude it and resubmit."
+    ci_summary "### :warning: Infrastructure fault — not a code or test failure\n\nNode \`${faulted_node:-unknown}\` could not run MFC (job \`$job_id\`). It is excluded and the job resubmitted elsewhere.\n"
     monitor_success=1
     exit 77
     ;;
-  78:*)
-    echo "Job $job_id skipped: a cluster-wide outage is already recorded."
-    monitor_success=1
-    exit 78
-    ;;
 esac
 
 # Check if job succeeded
 if [ "$exit_code" != "0:0" ]; then
   echo "ERROR: Job $job_id failed with exit code $exit_code"
+  # A GPU memory fault explains itself in a block the test harness prints; lift
+  # it onto the summary page so the faulting kernel and source line are visible
+  # without opening the log at all.
+  if grep -q 'GPU fault summary' "$output_file" 2>/dev/null; then
+    ci_summary "### GPU memory fault\n\n\`\`\`\n$(grep -A6 'GPU fault summary' "$output_file" | head -8 | sed 's/`/'"'"'/g')\n\`\`\`\n"
+  else
+    ci_summary "### Job \`$job_id\` failed (exit $exit_code)\n\n\`\`\`\n$(tail -n 15 "$output_file" | sed 's/`/'"'"'/g')\n\`\`\`\n"
+  fi
   exit 1
 fi
 

.github/scripts/classify-build-failure.sh

@@ -19,7 +19,6 @@
 # Exit codes:
 #   0   node looks healthy, carry on
 #   77  node-local fault -- caller should exclude this node and resubmit
-#   78  cluster-wide outage already recorded -- caller should skip, not requeue
 
 set -uo pipefail
 
@@ -33,7 +32,6 @@ fi
 
 EXIT_HEALTHY=0
 EXIT_NODE_FAULT=77
-EXIT_OUTAGE=78
 
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 node="${SLURMD_NODENAME:-$(hostname -s 2>/dev/null || hostname)}"
@@ -50,19 +48,6 @@ if [ -z "${SLURM_JOB_ID:-}" ]; then
     exit $EXIT_HEALTHY
 fi
 
-# --- Cluster-wide outage: requeuing cannot help, so skip rather than retry ---
-outage_rc=0
-bash "$SCRIPT_DIR/ci-outage.sh" check "$cluster" || outage_rc=$?
-if [ "$outage_rc" -eq 1 ]; then
-    echo "Preflight: skipping on $node because $cluster is known to be down."
-    exit $EXIT_OUTAGE
-elif [ "$outage_rc" -ne 0 ]; then
-    # Only exit 1 means "tripped". Anything else means the breaker could not be
-    # read at all (missing script, unreadable state dir), which says nothing
-    # about the cluster -- treating it as an outage would halt CI on a bug here.
-    echo "Preflight: could not read the outage breaker (exit $outage_rc); continuing."
-fi
-
 # --- Node health ---
 # Pick the *newest* install matching this job's device (build/install is named
 # e.g. gpu-acc-<hash>, gpu-mp-<hash>). Both halves matter: the device filter
@@ -114,7 +99,12 @@ echo "Preflight: probing $node with $syscheck_bin"
 # one there fails 127 no matter how healthy the node is. See
 # toolchain/templates/{phoenix,frontier,frontier_amd}.mako.
 case "$cluster" in
-    phoenix)               launcher=(mpirun -np 1) ;;
+    # --bind-to none: a single-rank health probe has nothing to bind against,
+    # and Open MPI's default binding fails outright on some Phoenix nodes
+    # ("hwloc_set_cpubind returned Error for bitmap 0"), killing the process
+    # before the binary is even launched. That is a launcher problem, not a
+    # node problem -- but it condemned three healthy nodes before being caught.
+    phoenix)               launcher=(mpirun --bind-to none -np 1) ;;
     frontier|frontier_amd) launcher=(srun -n1) ;;
     *)                     launcher=() ;;
 esac
@@ -131,18 +121,53 @@ fi
 # PMIX_ERR_NO_PERMISSIONS and friends from dstore_base.c are benign and appear
 # in more passing jobs than failing ones, so matching on log text would fail
 # healthy nodes.
-probe_rc=0
-if [ "${#launcher[@]}" -eq 0 ]; then
-    "$syscheck_bin" 2>&1 || probe_rc=$?
-else
-    "${launcher[@]}" "$syscheck_bin" 2>&1 || probe_rc=$?
-fi
+# Captured to a variable, not a temp file: this runs before any module set is
+# guaranteed and mktemp is not always on PATH here.
+run_probe() {
+    probe_rc=0
+    if [ "$#" -eq 0 ]; then
+        probe_out=$("$syscheck_bin" 2>&1) || probe_rc=$?
+    else
+        probe_out=$("$@" "$syscheck_bin" 2>&1) || probe_rc=$?
+    fi
+}
+
+run_probe "${launcher[@]}"
+
+# If this launcher does not take the flags we added, drop them and probe again
+# rather than reporting a verdict about the node. Otherwise a launcher that
+# rejects an option would fail every probe, and -- because a failed launch is
+# treated as inconclusive below -- would silently switch the preflight off
+# instead of failing loudly.
+case "$probe_out" in
+    *"unrecognized option"*|*"unrecognized argument"*|*"Unknown option"*|*"invalid option"*)
+        if [ "${#launcher[@]}" -gt 1 ]; then
+            echo "Preflight: ${launcher[0]} rejected the probe's options; retrying with none of them."
+            run_probe "${launcher[0]}"
+        fi
+        ;;
+esac
+
+printf '%s\n' "$probe_out"
 
 if [ "$probe_rc" -eq 0 ]; then
     echo "Preflight: $node passed."
     exit $EXIT_HEALTHY
 fi
 
+# Only a binary that RAN and failed says anything about this node. When the
+# launcher never got as far as starting it, the verdict is about mpirun or the
+# allocation, and excluding the node is both wrong and expensive -- three
+# healthy Phoenix nodes were excluded this way, two jobs deep, before the run
+# gave up. Judge nothing on a launch that never happened.
+case "$probe_out" in
+    *"The specified application failed to start"*|*"unable to start the specified application"*|*"was killed without launching the target application"*)
+        echo "Preflight: the launcher could not start $syscheck_bin on $node;"
+        echo "  that is a launcher or allocation problem, not evidence about the node. Continuing."
+        exit $EXIT_HEALTHY
+        ;;
+esac
+
 echo "::error::Preflight failed on $node: syscheck could not run MFC here."
 echo "This is an INFRASTRUCTURE fault, not a code or test failure."
 echo "MFC_FAULT_NODE=$node"