Merge remote-tracking branch 'origin/merge/master-0904' into up/mega

Author: sbryngelson

.claude/rules/common-pitfalls.md

@@ -74,6 +74,35 @@ covered in `docs/documentation/contributing.md`.
   sub-box, pack/unpack it with a device kernel (`s_l0_pack_unpack_block`,
   `s_amr_restrict_pack_device`) — that is why those exist. Measured: 10 of 60 covered
   cells delivered in the AMR cross-rank restrict, mass off 1.4e-5 per regrid.
+- An array whose bound is a device global (`dimension(num_fluids)`, `dimension(num_species)`) may be
+  passed to a device routine **from a parallel-loop body, but not from inside another
+  `GPU_ROUTINE(parallelism='[seq]')`**. CCE OpenACC rejects the second form with
+  `ftn-7066 ... Global in accelerator routine without declare -- num_fluids`, and reports it at
+  whatever line it gave up on: remove one trigger and the message *walks forward* to the next call,
+  so the reported line is not the cause. Only the plain lanes fail - under `--case-optimization`
+  those bounds are `parameter`s, so a green Case Opt lane beside a failing plain one is the
+  signature. Every accepted call site in the tree already obeys this (`m_cbc`, `m_ibm`,
+  `m_bubbles_EL`, `s_compute_cell_state`): form such a call in the loop body and pass scalars
+  deeper. Neither `cray_inline` nor a `num_fluids_max` bound nor dropping optional dummies helps -
+  all three were measured.
+- nvfortran 23.11/24.1 segfault (`fort2 TERMINATED by signal 11`) on a caller that passes a
+  `parameter` array from `m_thermochem` (e.g. `molecular_weights`) into a declare-target routine.
+  Read such arrays directly in the kernel, or pass a plain local computed from them.
+- The `USING_AMD` fypp guards (86 sites, `#:set` in `src/common/include/shared_parallel_macros.fpp`) are
+  load-bearing, not a stale workaround - do not "modernize" them away. They swap a device-global array
+  bound for a literal: `dimension(3)` for `num_dims`/`num_fluids` when case optimization is off (64
+  sites), and `dimension(20)` for `sys_size` in `m_compute_cbc` (21 sites, with a matching
+  `@:PROHIBIT` in `m_start_up` capping `sys_size <= 20` under AMD+CBC). Setting `USING_AMD = False`
+  and rebuilding amdflang `--gpu mp` without case optimization compiles CLEAN - 728 s, zero
+  diagnostics - and then NaNs at step 50 in CBC, riemann `wave_speeds=2`, IBM, surface tension,
+  QBMM/viscous and MHD HLLD, while both Lagrange bubble cases *complete* with out-of-tolerance
+  answers. Measured 2026-08-29 on MI210. A compile-only check returns green, so any future attempt to
+  drop these must run the tests, not just build.
+- The same "call it from the loop body" rule covers `m_thermochem`: calling `get_species_*` from
+  inside a `GPU_ROUTINE` rather than from the kernel gave CCE OpenMP a runtime
+  `Memory access fault by GPU node-N ... Reason: Unknown` on the first step (exit 134), while every
+  other backend ran. Evaluate them at the call site and pass the arrays in. Note this one only shows
+  at runtime, and only on a case that reaches the path - the build is clean.
 
 ## Parameters
 
@@ -143,6 +172,27 @@ covered in `docs/documentation/contributing.md`.
 - Tests are generated programmatically in `toolchain/mfc/test/cases.py` (parameter
   modifications on `BASE_CFG` via the `CaseGeneratorStack` push/pop pattern); test UUID =
   CRC32 of the trace string; `./mfc.sh test -l` lists all.
+- `--only` matches whole trace *elements*, not substrings, and `_filter_only`
+  (`toolchain/mfc/test/test.py`) **ANDs labels while ORing UUIDs**. So `--only bubbles` matches
+  nothing (the element is `Bubbles`), and `--only low_Mach=1 low_Mach=2` asks for cases carrying
+  both and also matches nothing. It then exits **143**, which reads like an external kill rather
+  than an empty filter. Pass UUIDs whenever you want the union of several groups.
+- Sibling `define_case_d` calls off the same stack level are never *combined*. Two switches that
+  only matter together (`avg_state=1` needs `wave_speeds=2` to be read at all) therefore get zero
+  effective coverage unless something pushes one and defines the other beneath it. Check
+  reachability before trusting that a flag is tested.
+- `--no-build` silently runs whatever binary is on disk for a configuration it did not build.
+  Chemistry has its own config (`gpu-mp-chem-*`) that a plain `./mfc.sh build` never produces, so
+  a `--no-build` run reports failures from stale binaries and hides real compile breaks. Run
+  chemistry-touching sets without it.
+- Pick the newest binary by the *binary's* mtime (`ls -t build/install/*/bin/simulation`), not the
+  install directory's - a stale config's directory can be newer than a fresh build's.
+- The pre-commit hook lives in the main repo's `.git/hooks/` and git exports `GIT_DIR` there
+  during a commit, so from a worktree the toolchain lint enumerates the *other* checkout and
+  fails. Reproduce with `GIT_DIR=<main>/.git ./mfc.sh precheck`. Run precheck by hand and commit
+  with `--no-verify`.
+- `/tmp` is node-local: scratch does not survive a compute-node change, and its absence is
+  silence, not an error. Keep patches and resource baselines on a shared filesystem.
 - Golden files are tolerance-compared. Regenerate only the affected tests
   (`./mfc.sh test --generate --only <tests>`) — an unexplained golden-file diff is a bug
   report, not noise to be regenerated away.

.github/scripts/check_coverage_map_health.py

@@ -1,12 +1,10 @@
 """Fail loudly if the committed coverage map is stale or under-covers. Used by coverage-health.yml."""
 import datetime
-import os
-import subprocess
 import sys
 from pathlib import Path
 
 sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "toolchain"))
-from mfc.test.coverage import COVERAGE_MAP_PATH, load_map, map_health  # noqa: E402
+from mfc.test.coverage import COVERAGE_MAP_PATH, _git, load_map, map_health  # noqa: E402
 from mfc.test.cases import list_cases  # noqa: E402  (returns the current test list)
 
 MAX_AGE_DAYS = 10
@@ -33,18 +31,6 @@
 COVERAGE_RELEVANT_PATHS = [":(glob)src/**/*.fpp", "toolchain/mfc/test/cases.py"]
 
 
-def _git_env():
-    """The environment minus every GIT_* variable.
-
-    `cwd` is how callers select the repository here, but git honors an inherited GIT_DIR /
-    GIT_INDEX_FILE over the working directory -- and git exports both while running a hook.
-    MFC's pre-commit hook runs precheck, whose toolchain lint imports this script and calls
-    these functions against throwaway test repos; without the scrub those calls silently
-    answer for the developer's real repository instead.
-    """
-    return {k: v for k, v in os.environ.items() if not k.startswith("GIT_")}
-
-
 def verified_sha(cwd=None):
     """Commit the last successful refresh verified the map against, or None if unknown.
 
@@ -53,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 = subprocess.run(["git", "rev-parse", "--verify", "--quiet", f"{VERIFIED_REF}^{{commit}}"], capture_output=True, text=True, check=False, cwd=cwd, env=_git_env())
+    rev = _git(["rev-parse", "--verify", "--quiet", f"{VERIFIED_REF}^{{commit}}"], cwd)
     return rev.stdout.strip() or None
 
 
@@ -66,10 +52,10 @@ def verified_after_last_change(git_sha, cwd=None):
     """
     if not git_sha:
         return None
-    last = subprocess.run(["git", "log", "-1", "--format=%H", "--", *COVERAGE_RELEVANT_PATHS], capture_output=True, text=True, check=False, cwd=cwd, env=_git_env())
+    last = _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 = subprocess.run(["git", "merge-base", "--is-ancestor", last.stdout.strip(), git_sha], capture_output=True, check=False, cwd=cwd, env=_git_env())
+    ancestor = _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

@@ -0,0 +1,118 @@
+#!/bin/bash
+# Per-cluster circuit breaker for environment-wide outages.
+#
+# Some failures are not the node's fault and not the code's fault: pypi.org
+# unreachable from a login node, a module tree mid-upgrade, a full project
+# filesystem. Requeuing elsewhere cannot help, and every job that starts pays
+# the same discovery cost -- on 2026-08-28, 17 Frontier jobs each spent ~33
+# minutes learning that PyPI was down.
+#
+# The first job to notice records a marker on the shared filesystem (every
+# self-hosted runner for a cluster shares $HOME); later jobs check it and exit
+# immediately instead of submitting a SLURM job that is going to fail.
+#
+# The breaker is deliberately self-healing. A marker expires after
+# MFC_CI_OUTAGE_TTL_SECONDS, and a marker that cannot be parsed is ignored, so
+# neither a stale file nor a truncated write can wedge CI. Only jobs that
+# actually observe the outage re-mark it, so once the outage clears the breaker
+# closes on its own.
+#
+# Usage:
+#   ci-outage.sh mark  <cluster> <reason>   record an outage
+#   ci-outage.sh check <cluster>            exit 0 = clear, 1 = outage active
+#   ci-outage.sh clear <cluster>            reset the breaker
+#
+# Env:
+#   MFC_CI_STATE_DIR             where markers live (default ~/.mfc-ci-state)
+#   MFC_CI_OUTAGE_TTL_SECONDS    marker lifetime in seconds (default 1200)
+
+set -uo pipefail
+
+STATE_DIR="${MFC_CI_STATE_DIR:-$HOME/.mfc-ci-state}"
+TTL="${MFC_CI_OUTAGE_TTL_SECONDS:-1200}"
+
+# A non-numeric TTL would make the age comparison below emit "integer expression
+# expected" and exit with a code the caller reads as neither clear nor tripped.
+# Fall back to the default rather than letting a typo gate CI.
+case "$TTL" in
+    ''|*[!0-9]*)
+        echo "Ignoring non-numeric MFC_CI_OUTAGE_TTL_SECONDS='$TTL'; using 1200." >&2
+        TTL=1200
+        ;;
+esac
+
+EXIT_CLEAR=0
+EXIT_TRIPPED=1
+EXIT_USAGE=2
+
+usage() {
+    echo "Usage: $0 {mark <cluster> <reason>|check <cluster>|clear <cluster>}" >&2
+}
+
+# Keep the marker name filesystem-safe regardless of what the caller passes.
+marker_for() {
+    local cluster
+    cluster=$(printf '%s' "$1" | tr -c 'A-Za-z0-9_.-' '_')
+    printf '%s/outage-%s' "$STATE_DIR" "$cluster"
+}
+
+cmd="${1:-}"
+cluster="${2:-}"
+
+if [ -z "$cmd" ] || [ -z "$cluster" ]; then
+    usage
+    exit $EXIT_USAGE
+fi
+
+marker=$(marker_for "$cluster")
+
+case "$cmd" in
+    mark)
+        reason="${3:-unspecified}"
+        mkdir -p "$STATE_DIR" || exit $EXIT_USAGE
+        # Write to a temporary file and rename so a concurrent `check` never
+        # observes a half-written marker.
+        tmp="${marker}.$$.tmp"
+        {
+            date +%s
+            printf '%s\n' "$reason"
+        } > "$tmp" && mv -f "$tmp" "$marker"
+        echo "Recorded $cluster outage: $reason"
+        echo "  marker: $marker (expires after ${TTL}s)"
+        ;;
+
+    check)
+        [ -f "$marker" ] || exit $EXIT_CLEAR
+
+        stamp=$(head -n1 "$marker" 2>/dev/null)
+        reason=$(tail -n +2 "$marker" 2>/dev/null)
+
+        # A marker we cannot parse is treated as absent: an unreadable breaker
+        # must never be an un-clearable one.
+        case "$stamp" in
+            ''|*[!0-9]*)
+                echo "Ignoring unparseable outage marker $marker"
+                exit $EXIT_CLEAR
+                ;;
+        esac
+
+        age=$(( $(date +%s) - stamp ))
+        if [ "$age" -ge "$TTL" ] || [ "$age" -lt 0 ]; then
+            exit $EXIT_CLEAR
+        fi
+
+        echo "::warning::Skipping: known $cluster outage recorded ${age}s ago: ${reason:-unspecified}"
+        echo "Clear it early by deleting $marker"
+        exit $EXIT_TRIPPED
+        ;;
+
+    clear)
+        rm -f "$marker"
+        echo "Cleared any $cluster outage marker ($marker)"
+        ;;
+
+    *)
+        usage
+        exit $EXIT_USAGE
+        ;;
+esac

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

@@ -0,0 +1,41 @@
+#!/bin/bash
+# Decide whether a failed build was a cluster-wide dependency outage.
+#
+# MFC bootstraps its Python toolchain into build/venv on the first ./mfc.sh call
+# of a job, pulling from pypi.org. On Phoenix clean_build has just moved build/
+# aside, so that happens every time; on Frontier it happens in the login-node
+# "Fetch Dependencies" step. When the index is unreachable the build fails for a
+# reason no other node improves on, so it is worth recording once and skipping
+# the rest of the matrix rather than having each job spend ~33 minutes
+# rediscovering it (17 Frontier jobs did exactly that on 2026-08-28).
+#
+# Usage: classify-build-failure.sh <logfile> <cluster>
+#
+# Exit codes:
+#   78  cluster-wide dependency outage; it has been recorded
+#   0   ordinary build failure, caller should keep its own exit code
+
+set -uo pipefail
+
+log="${1:-}"
+cluster="${2:-}"
+
+if [ -z "$log" ] || [ -z "$cluster" ]; then
+    echo "Usage: $0 <logfile> <cluster>" >&2
+    exit 0
+fi
+
+# No log means nothing to classify. Never claim an outage on absent evidence.
+[ -f "$log" ] || exit 0
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+
+# The URL may or may not be wrapped (uv quotes it in backticks, plain pip does
+# not), so do not require a character between the colon and the scheme.
+if grep -qE "Failed to fetch:[^h]*https?://pypi|uv install failed|\(venv\) Installation failed" "$log"; then
+    bash "$SCRIPT_DIR/ci-outage.sh" mark "$cluster" \
+        "PyPI/uv dependency install failed during build"
+    exit 78
+fi
+
+exit 0

.github/scripts/monitor_slurm_job.sh

@@ -99,6 +99,11 @@ if ! [[ "$SLURM_MAX_QUEUE_SECONDS" =~ ^[0-9]+$ ]]; then
   echo "ERROR: SLURM_MAX_QUEUE_SECONDS must be a non-negative integer (seconds), got '$SLURM_MAX_QUEUE_SECONDS'" >&2
   exit 1
 fi
+# How long to wait between status polls and between output-stabilization
+# checks. Overridable so tests can exercise this script without sleeping
+# through it; CI leaves it at the default.
+: "${MFC_MONITOR_POLL_SECONDS:=5}"
+
 queue_start=$(date +%s)
 
 abort_queue_starvation() {
@@ -142,7 +147,7 @@ while [ ! -f "$output_file" ]; do
       ;;
     PENDING|CONFIGURING)
       unknown_count=0
-      sleep 5
+      sleep "$MFC_MONITOR_POLL_SECONDS"
       ;;
     RUNNING|COMPLETING)
       unknown_count=0
@@ -155,7 +160,7 @@ while [ ! -f "$output_file" ]; do
       if [ $((unknown_count % 12)) -eq 1 ]; then
         echo "Warning: Could not query job $job_id state (SLURM may be temporarily unavailable)..."
       fi
-      sleep 5
+      sleep "$MFC_MONITOR_POLL_SECONDS"
       ;;
     *)
       # Terminal state — job finished without creating output
@@ -164,7 +169,7 @@ while [ ! -f "$output_file" ]; do
         exit 1
       fi
       # Unrecognized state, keep waiting
-      sleep 5
+      sleep "$MFC_MONITOR_POLL_SECONDS"
       ;;
   esac
 done
@@ -205,7 +210,7 @@ while true; do
     last_heartbeat=$current_time
   fi
 
-  sleep 5
+  sleep "$MFC_MONITOR_POLL_SECONDS"
 done
 
 # Give tail a moment to flush the final lines, then stop streaming.
@@ -229,7 +234,7 @@ if [ -f "$output_file" ]; then
     if [ $same_count -ge 2 ]; then
       break
     fi
-    sleep 5
+    sleep "$MFC_MONITOR_POLL_SECONDS"
   done
 fi
 
@@ -262,6 +267,23 @@ 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).
+case "$exit_code" in
+  77:*)
+    echo "Job $job_id failed preflight: the node is unusable — signaling caller to exclude it and resubmit."
+    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"