Merge branch 'master' into fix/spelling-typos-1498

Author: engineer-scientist

.claude/rules/common-pitfalls.md

@@ -14,7 +14,7 @@ covered in `docs/documentation/contributing.md`.
   bubbles and IB. Read that routine for the current value rather than assuming one.
 - Riemann solvers: left state at `j`, right state at `j+1`.
 - All equation indices live in the `eqn_idx` struct (`eqn_idx_info` in
-  `m_derived_types.fpp`, populated in `m_global_parameters.fpp`): `%cont`, `%mom`, `%E`,
+  `m_derived_types.fpp`, populated by `s_initialize_eqn_idx` in `m_global_parameters_common.fpp`): `%cont`, `%mom`, `%E`,
   `%adv`, plus optional ranges (`%bub`, `%stress`, `%species`, `%B`, ...). The old
   `contxb`/`momxb` shorthands are gone. Index positions depend on `model_eqns` and
   enabled features — changing either moves ALL indices; never hard-code one.
@@ -42,17 +42,40 @@ covered in `docs/documentation/contributing.md`.
 - Adding one: `_r()` definition + `_nv()` `NAMELIST_VARS` registration in
   `toolchain/mfc/params/definitions.py`; `case_validator.py` only if physics-constrained
   (with a `PHYSICS_DOCS` entry). Fortran declarations and namelist bindings are
-  auto-generated at CMake configure time — re-run cmake (or `./mfc.sh build`) after editing.
-- Still manual: array variables and derived-type members (declare in
-  `src/*/m_global_parameters.fpp` / the relevant type), and case-optimization parameters
-  (`CASE_OPT_PARAMS` + the `#:else` block in `src/simulation/m_global_parameters.fpp`).
-  Gotcha: under `--case-optimization` those are baked into the binary and dropped from
-  the namelist, so changing one needs a *rebuild*, not a case edit.
+  auto-generated at build time (ninja-tracked custom command) — re-run cmake (or `./mfc.sh build`) after editing.
+- Still manual: derived-type `TYPE` member definitions in `src/common/m_derived_types.fpp`;
+  default-value assignments in `s_assign_default_values_to_user_inputs`; the
+  `CASE_OPT_EXTRA_LINES` literal in `toolchain/mfc/params/generators/fortran_gen.py` (covers `num_dims`,
+  `num_vels`, `weno_polyn`, `muscl_polyn`, `weno_num_stencils`, `wenojs`);
+  multi-variable declaration lines (`bc_x/y/z`, `x/y/z_domain`, `x/y/z_output`, post's
+  `G`); and the MPI broadcast residue in `m_mpi_proxy` (computed variables that are not
+  namelist-bound: `m_glb`/`n_glb`/`p_glb`, `cfl_dt`, `bc_io`, and complex struct-member
+  array loops — these cannot be auto-generated and stay hand-listed). Everything else — scalar declarations, plain arrays (`FORTRAN_ARRAY_DIMS`
+  table in `definitions.py`), derived-type namelist declarations including `GPU_DECLARE`
+  lines and Doxygen descs (`TYPED_DECLS` table in `definitions.py`), the simulation
+  case-optimization declaration block, and the per-target MPI broadcast lists for all
+  namelist-registry scalars (`generated_bcast.fpp`) — is regenerated at build time by a
+  ninja-tracked custom command (editing `params/*.py` triggers regeneration automatically).
+  Gotcha: ADDING a new file under `toolchain/mfc/params/` needs one reconfigure
+  (the custom command's DEPENDS list is globbed at configure time). Under `--case-optimization` the baked-in constants are dropped from the
+  namelist, so changing one needs a *rebuild*, not a case edit.
+- Shared-state pattern: namelist declarations (`#:include 'generated_decls.fpp'`), the
+  `eqn_idx`/`sys_size`/`b_size`/`tensor_size` state variables, and the common defaults
+  core all live in `src/common/m_global_parameters_common.fpp`. Each per-target
+  `m_global_parameters.fpp` does `use m_global_parameters_common` (default-public), so
+  `use m_global_parameters` continues to work for all downstream modules without change.
+  Sim-only declarations (GPU_DECLARE, Re_idx allocation) stay in
+  `m_global_parameters_common` behind `#ifdef MFC_SIMULATION`. Generated includes
+  (`generated_decls.fpp`, `generated_bcast.fpp`, `generated_case_opt_decls.fpp`) must exist for every target — the build
+  emits stubs where the content is sim-only, so a common file that includes one will
+  compile for pre/post too.
 - Runtime checks (`@:PROHIBIT`) go where they run: shared →
   `src/common/m_checker_common.fpp`; simulation-only → `src/simulation/m_checker.fpp`;
   pre/post-only → `src/{pre,post}_process/m_checker.fpp` (their `s_check_inputs` are
   currently empty — that IS the right place, not m_checker_common).
-- Analytic ICs are compiled into the binary (syntax error = build failure, not runtime).
+- Analytic ICs are compiled into the binary. Expressions are AST-validated at case load
+  (syntax errors and unknown variables are immediate, named errors; bare `e` is not a
+  variable — write `exp(1.0)`).
   Each IC variable maps to an `eqn_idx%…` expression in `QPVF_IDX_VARS`
   (`toolchain/mfc/case.py`); a new patch-settable conserved variable means updating that
   map AND the Fortran `eqn_idx` builder to agree — a mismatch is a silent wrong index.

.github/scripts/prebuild-case-optimization.sh

@@ -22,12 +22,44 @@ case "$cluster" in
     *) echo "ERROR: Unknown cluster '$cluster'"; exit 1 ;;
 esac
 
+# Optional sharding (format "i/N", e.g. "1/2"), set by submit-slurm-job.sh's
+# [shard] argument via $job_shard: shard i builds every Nth case of the sorted
+# case list. Unset = build all cases in one job (default; other clusters).
+shard="${job_shard:-}"
+if [ -n "$shard" ]; then
+    # Validate full shape: must be exactly "digits/digits" — one slash with
+    # non-empty, purely numeric, non-leading-zero parts on both sides.
+    # Split first, then validate each part independently so that inputs like
+    # "1/" "/2" "//" "1/2/3" "a/b" "12" are all caught before any arithmetic.
+    shard_idx="${shard%%/*}"
+    shard_count="${shard##*/}"
+    # Reject if no slash (idx and count are equal and equal to the whole string)
+    case "$shard_idx" in
+        ''|*[!0-9]*|0*) echo "ERROR: bad shard '$shard' (expected i/N)"; exit 1 ;;
+    esac
+    case "$shard_count" in
+        ''|*[!0-9]*|0*) echo "ERROR: bad shard '$shard' (expected i/N)"; exit 1 ;;
+    esac
+    # Confirm the string is exactly "idx/count" — catches "12" (no slash) and
+    # "1/2/3" (extra slash, where idx=1 and count=2/3 would have failed above,
+    # but this is an extra safety net).
+    if [ "$shard" != "$shard_idx/$shard_count" ]; then
+        echo "ERROR: bad shard '$shard' (expected i/N)"; exit 1
+    fi
+    if [ "$shard_idx" -lt 1 ] || [ "$shard_idx" -gt "$shard_count" ]; then
+        echo "ERROR: bad shard '$shard' (expected i/N with 1 <= i <= N)"; exit 1
+    fi
+fi
+
 # Phoenix starts fresh (no prior dep build); other clusters pre-build deps via
 # build.sh first, so we must preserve them and only clean MFC target staging.
+# Sharded jobs share one workspace and run concurrently, so the workflow
+# cleans once before submitting them — cleaning here would wipe a sibling
+# shard's in-progress build.
 if [ "$cluster" = "phoenix" ]; then
     source .github/scripts/clean-build.sh
     clean_build
-else
+elif [ -z "$shard" ]; then
     find build/staging -maxdepth 1 -regex '.*/[0-9a-f]+' -type d -exec rm -rf {} + 2>/dev/null || true
     find build/install -maxdepth 1 -regex '.*/[0-9a-f]+' -type d -exec rm -rf {} + 2>/dev/null || true
 fi
@@ -40,7 +72,49 @@ case "$job_interface" in
     *)   echo "ERROR: prebuild requires gpu interface (acc or omp)"; exit 1 ;;
 esac
 
+# Case-optimized simulation builds land in per-case hash-named staging dirs,
+# but syscheck/pre_process/post_process hash identically across these cases.
+# Concurrent shards must not build those shared staging dirs simultaneously:
+# shard 1 builds them first and drops a done marker; other shards wait for it,
+# after which their builds no-op in the shared dirs.
+if [ -n "$shard" ] && [ "$shard_count" -gt 1 ]; then
+    shared_marker_done="build/.prebuild-shared-targets-done"
+    shared_marker_failed="build/.prebuild-shared-targets-failed"
+    set -- benchmarks/*/case.py
+    first_case="$1"
+    if [ "$shard_idx" -eq 1 ]; then
+        # Remove both markers at the start so reruns and manual invocations
+        # never observe stale state from a prior run.
+        rm -f "$shared_marker_done" "$shared_marker_failed"
+        echo "=== Shard 1/$shard_count: building shared targets ==="
+        # Write the failure marker if the build exits non-zero so other shards
+        # can detect the failure immediately instead of waiting 90 minutes.
+        trap 'touch "$shared_marker_failed"' ERR
+        ./mfc.sh build -i "$first_case" -t syscheck pre_process post_process --case-optimization $gpu_opts -j 8
+        trap - ERR
+        touch "$shared_marker_done"
+    else
+        echo "=== Shard $shard_idx/$shard_count: waiting for shard 1 to build shared targets ==="
+        waited=0
+        until [ -f "$shared_marker_done" ]; do
+            if [ -f "$shared_marker_failed" ]; then
+                echo "ERROR: shard 1 failed to build shared targets; see shard 1 log"; exit 1
+            fi
+            if [ "$waited" -ge 5400 ]; then
+                echo "ERROR: timed out waiting for $shared_marker_done"; exit 1
+            fi
+            sleep 30
+            waited=$((waited + 30))
+        done
+    fi
+fi
+
+idx=0
 for case in benchmarks/*/case.py; do
+    idx=$((idx + 1))
+    if [ -n "$shard" ] && [ $(((idx - 1) % shard_count)) -ne $((shard_idx - 1)) ]; then
+        continue
+    fi
     echo "=== Pre-building: $case ==="
     ./mfc.sh run "$case" --case-optimization $gpu_opts -j 8 --dry-run
 done

.github/scripts/run_parallel_benchmarks.sh

@@ -62,6 +62,12 @@ bash "${SCRIPT_DIR}/run_monitored_slurm_job.sh" "$pr_job_id" "pr/${job_slug}.out
 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
 else
     echo "PR job completed successfully"
 fi

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

@@ -52,7 +52,10 @@ case "$cluster" in
         account="CFD154"
         job_prefix="MFC"
         qos="hackathon"
-        extra_sbatch=""
+        # Let each job's slurmstepd broker its own steps instead of routing
+        # every srun through slurmctld. The in-job test suite launches ~1700+
+        # srun steps per allocation, which congests the Frontier controller.
+        extra_sbatch="#SBATCH --stepmgr"
         test_time="01:59:00"
         bench_time="01:59:00"
         gpu_partition_dynamic=false
@@ -62,7 +65,7 @@ case "$cluster" in
         account="CFD154"
         job_prefix="MFC"
         qos="hackathon"
-        extra_sbatch=""
+        extra_sbatch="#SBATCH --stepmgr"
         test_time="01:59:00"
         bench_time="01:59:00"
         gpu_partition_dynamic=false

.github/workflows/test.yml

@@ -156,9 +156,19 @@ jobs:
       # Replaces the container: directive so we can free disk space first.
       # Uses "docker run -d ... sleep infinity" + "docker exec" to preserve
       # installed packages and env vars across steps.
+      # Retry the pull: nvcr.io intermittently times out ("context deadline
+      # exceeded") under load, and ~30 matrix jobs hit it at once. Pulls
+      # resume completed layers, so retries are cheap.
       - name: Pull NVHPC container
         if:   matrix.nvhpc
-        run:  docker pull "$NVHPC_IMAGE"
+        run: |
+          for attempt in 1 2 3 4 5; do
+            docker pull "$NVHPC_IMAGE" && exit 0
+            echo "docker pull failed (attempt $attempt/5); retrying in $((attempt * 30))s..."
+            sleep $((attempt * 30))
+          done
+          echo "::error::Failed to pull $NVHPC_IMAGE after 5 attempts"
+          exit 1
 
       - name: Start NVHPC container
         if:   matrix.nvhpc
@@ -392,6 +402,13 @@ jobs:
             cluster_name: 'Oak Ridge | Frontier (AMD)'
             device: 'cpu'
             interface: 'none'
+            shard: '1/2'
+          - runner:       'frontier'
+            cluster:      'frontier_amd'
+            cluster_name: 'Oak Ridge | Frontier (AMD)'
+            device: 'cpu'
+            interface: 'none'
+            shard: '2/2'
     runs-on:
       group:  phoenix
       labels: ${{ matrix.runner }}
@@ -410,7 +427,7 @@ jobs:
 
       - name: Fetch Dependencies
         if:   matrix.cluster != 'phoenix'
-        timeout-minutes: 60
+        timeout-minutes: 120
         run:  bash .github/workflows/${{ matrix.cluster }}/build.sh ${{ matrix.device }} ${{ matrix.interface }}
 
       - name: Build
@@ -513,7 +530,22 @@ jobs:
 
       - name: Pre-Build (SLURM)
         if:   matrix.cluster == 'frontier_amd'
-        run:  bash .github/scripts/submit-slurm-job.sh .github/scripts/prebuild-case-optimization.sh gpu ${{ matrix.interface }} ${{ matrix.cluster }}
+        # AMD flang is slow enough that one serial pre-build job exceeds its
+        # walltime, so split the case list across two concurrent SLURM jobs.
+        # The shards share this workspace and skip their in-job staging clean,
+        # so clean once here on the login node before submitting.
+        run:  |
+          find build/staging -maxdepth 1 -regex '.*/[0-9a-f]+' -type d -exec rm -rf {} + 2>/dev/null || true
+          find build/install -maxdepth 1 -regex '.*/[0-9a-f]+' -type d -exec rm -rf {} + 2>/dev/null || true
+          rm -f build/.prebuild-shared-targets-done
+          bash .github/scripts/submit-slurm-job.sh .github/scripts/prebuild-case-optimization.sh gpu ${{ matrix.interface }} ${{ matrix.cluster }} 1/2 &
+          pid1=$!
+          bash .github/scripts/submit-slurm-job.sh .github/scripts/prebuild-case-optimization.sh gpu ${{ matrix.interface }} ${{ matrix.cluster }} 2/2 &
+          pid2=$!
+          rc=0
+          wait "$pid1" || rc=1
+          wait "$pid2" || rc=1
+          exit $rc
 
       - name: Build & Run Case-Optimization Tests
         if:   matrix.cluster != 'phoenix' && matrix.cluster != 'frontier_amd'
@@ -536,6 +568,8 @@ jobs:
         if:   always()
         run:  |
           for f in prebuild-case-optimization-${{ matrix.device }}-${{ matrix.interface }}.out \
+                   prebuild-case-optimization-${{ matrix.device }}-${{ matrix.interface }}-1-of-2.out \
+                   prebuild-case-optimization-${{ matrix.device }}-${{ matrix.interface }}-2-of-2.out \
                    run-case-optimization-${{ matrix.device }}-${{ matrix.interface }}.out; do
             [ -f "$f" ] && echo "=== $f ===" && cat "$f"
           done
@@ -546,5 +580,5 @@ jobs:
         with:
           name: case-opt-${{ strategy.job-index }}-${{ matrix.cluster }}-${{ matrix.interface }}
           path: |
-            prebuild-case-optimization-${{ matrix.device }}-${{ matrix.interface }}.out
+            prebuild-case-optimization-${{ matrix.device }}-${{ matrix.interface }}*.out
             run-case-optimization-${{ matrix.device }}-${{ matrix.interface }}.out