Merging upstream

Author: Dimitrios Adam

.claude/rules/common-pitfalls.md

@@ -4,7 +4,9 @@
 - Grid dimensions: `m`, `n`, `p` (cells in x, y, z). 1D: n=p=0. 2D: p=0.
 - Interior domain: `0:m`, `0:n`, `0:p`
 - Buffer/ghost region: `-buff_size:m+buff_size` (similar for n, p in multi-D)
-- `buff_size` depends on WENO order and features (typically `2*weno_polyn + 2`)
+- `buff_size` is **not** a single formula: it's set per reconstruction scheme (WENO/MUSCL/IGR) in
+  `s_configure_coordinate_bounds` (`m_helper_basic.fpp`) and floored higher for Lagrange bubbles and IB.
+  Read that routine for the current value rather than assuming one.
 - Domain bounds: `idwint(1:3)` (interior `0:m`), `idwbuff(1:3)` (with ghost cells)
 - Cell-center coords: `x_cc(-buff_size:m+buff_size)`, `y_cc(...)`, `z_cc(...)`
 - Cell-boundary coords: `x_cb(-1-buff_size:m+buff_size)`
@@ -38,8 +40,8 @@
 - Boundary condition symmetry requirements must be maintained
 
 ## Compiler-Specific Issues
-- CI-gated compilers (must always pass): gfortran, nvfortran, Cray ftn, and Intel ifx
-- AMD flang is additionally supported for `--gpu mp` builds but not in the CI matrix
+- See the compiler-backend matrix in `.claude/rules/gpu-and-mpi.md` for which compilers
+  are CI-gated and which backends each supports.
 - Each compiler has different strictness levels and warning behavior
 - Fypp macros must expand correctly for both GPU and CPU builds
 
@@ -54,12 +56,8 @@
 - Do not regenerate ALL golden files unless you understand every output change
 
 ## PR Checklist
-Before submitting a PR:
-- [ ] `./mfc.sh format -j 8` (auto-format)
-- [ ] `./mfc.sh precheck -j 8` (5 CI lint checks)
-- [ ] `./mfc.sh build -j 8` (compiles)
-- [ ] `./mfc.sh test --only <relevant> -j 8` (tests pass)
-- [ ] If adding parameters: all 4 locations updated
+The base loop (format → precheck → build → test → one logical commit) is the
+Development Workflow Contract in `CLAUDE.md`. Beyond it, watch for:
+- [ ] If adding parameters: definitions.py (_r + _nv) updated; cmake reconfigured; case_validator.py if constraints
 - [ ] If modifying `src/common/`: all three targets tested
-- [ ] If changing output: golden files regenerated for affected tests
-- [ ] One logical change per commit
+- [ ] If changing output: golden files regenerated for affected tests (`./mfc.sh test --generate --only <tests>`)

.claude/rules/fortran-conventions.md

@@ -15,11 +15,7 @@ Every Fortran module follows this pattern:
 - Finalization subroutine: `s_finalize_<feature>_module`
 
 ## Naming
-- Modules: `m_<feature>`
-- Public subroutines: `s_<verb>_<noun>`
-- Public functions: `f_<verb>_<noun>`
-- Private/local variables: no prefix required
-- Constants: descriptive names, not ALL_CAPS
+See "Naming Conventions" in `CLAUDE.md`.
 
 ## Forbidden Patterns
 
@@ -57,10 +53,8 @@ Enforced by convention/code review (not automated):
 - Fortran-side runtime validation also exists in `m_checker*.fpp` files using `@:PROHIBIT`
 
 ## Precision Types
-- `wp` (working precision): used for computation. Double by default.
-- `stp` (storage precision): used for field data arrays and I/O. Double by default.
-- In single-precision mode (`--single`): both become single.
-- In mixed-precision mode (`--mixed`): wp=double, stp=half.
+`wp`/`stp` are defined in `CLAUDE.md` (`wp` = computation, `stp` = field-data storage + I/O). Detail:
+- Modes: default both double; `--single` → both single; `--mixed` → wp=double, stp=half.
 - MPI type matching: `mpi_p` must match `wp`, `mpi_io_p` must match `stp`.
 - Always use generic intrinsics: `sqrt` not `dsqrt`, `abs` not `dabs`.
 - Cast with `real(..., wp)` or `real(..., stp)`, never `dble(...)`.

.claude/rules/gpu-and-mpi.md

@@ -23,26 +23,17 @@ compiles to either OpenACC or OpenMP target offload depending on the build flag:
 
 ### Key GPU Macros (always use the `GPU_*` prefix)
 
-Inline macros (use `$:` prefix):
-- `$:GPU_PARALLEL_LOOP(collapse=N, private=[...], reduction=[...], reductionOp='+')` —
-  Parallel loop over GPU threads. Most common GPU macro.
-- `$:END_GPU_PARALLEL_LOOP()` — Required closing for GPU_PARALLEL_LOOP.
-- `$:GPU_LOOP(collapse=N, ...)` — Inner loop within a GPU parallel region.
-- `$:GPU_ENTER_DATA(create=[...])` — Allocate device memory (unscoped).
-- `$:GPU_EXIT_DATA(delete=[...])` — Free device memory.
-- `$:GPU_UPDATE(host=[...])` — Copy device → host (before MPI send).
-- `$:GPU_UPDATE(device=[...])` — Copy host → device (after MPI receive).
-- `$:GPU_ROUTINE(parallelism='[seq]')` — Mark routine for device compilation.
-- `$:GPU_DECLARE(create=[...])` — Declare device-resident data.
-- `$:GPU_ATOMIC(atomic='update')` — Atomic operation on device.
-- `$:GPU_WAIT()` — Synchronization barrier.
-
-Block macros (use `#:call`/`#:endcall`):
-- `GPU_PARALLEL(...)` — GPU parallel region (used for scalar reductions like `maxval`/`minval`).
-- `GPU_DATA(copy=..., create=..., ...)` — Scoped data region.
-- `GPU_HOST_DATA(use_device_addr=[...])` — Host code with device pointers.
-
-Typical GPU loop pattern (used 750+ times in the codebase):
+Full set with signatures in `parallel_macros.fpp`. The ones you reach for most:
+- `$:GPU_PARALLEL_LOOP(collapse=N, private=[...], reduction=[...], reductionOp='+')`
+  + `$:END_GPU_PARALLEL_LOOP()` — parallel spatial loop; by far the most common (see pattern below).
+- `$:GPU_LOOP(collapse=N, ...)` — inner loop *within* a parallel region.
+- `$:GPU_UPDATE(host=[...])` / `$:GPU_UPDATE(device=[...])` — device↔host copies (around MPI; see below).
+- `#:call GPU_PARALLEL(...)` — block region for scalar reductions (`maxval`/`minval`).
+
+Others in `parallel_macros.fpp`: `GPU_ENTER_DATA`/`GPU_EXIT_DATA`, `GPU_DECLARE`, `GPU_ROUTINE`,
+`GPU_ATOMIC`, `GPU_WAIT`, and the block macros `GPU_DATA`, `GPU_HOST_DATA`.
+
+Typical GPU loop pattern (the dominant spatial-loop idiom):
 ```
 $:GPU_PARALLEL_LOOP(private='[i,j,k,l]', collapse=3)
 do l = idwbuff(3)%beg, idwbuff(3)%end
@@ -108,16 +99,10 @@ Use `#ifdef` for feature, target, compiler, and library gating:
 - `MFC_POST_PROCESS` — Only in post_process builds
 
 ### Compiler gating (for compiler-specific workarounds)
-- `_CRAYFTN` — Cray Fortran compiler
-- `__NVCOMPILER_GPU_UNIFIED_MEM` — NVIDIA unified memory (GH-200 / `--unified`)
-- `__PGI` — Legacy PGI/NVIDIA compiler
-- `__INTEL_COMPILER` — Intel compiler
-- `FRONTIER_UNIFIED` — Frontier HPC unified memory
-
-### Library-specific code
-- FFTW (`m_fftw.fpp`) uses heavy `#ifdef` gating for `MFC_GPU` and `__PGI`
-- CUDA Fortran (`cudafor` module) is gated behind `__NVCOMPILER_GPU_UNIFIED_MEM`
-- SILO/HDF5 interfaces may have conditional paths
+Compiler/feature macros: `_CRAYFTN`, `__NVCOMPILER_GPU_UNIFIED_MEM` (NVIDIA unified mem, GH-200 /
+`--unified`), `__PGI` (legacy PGI/NVIDIA), `__INTEL_COMPILER`, `FRONTIER_UNIFIED`. Library code is
+similarly gated (FFTW in `m_fftw.fpp` on `MFC_GPU`/`__PGI`; CUDA Fortran `cudafor` on
+`__NVCOMPILER_GPU_UNIFIED_MEM`; SILO/HDF5 paths). Grep the relevant file for exact usage.
 
 When adding new `#ifdef` blocks, always provide an `#else` or `#endif` path so
 the code compiles in all configurations (CPU-only, GPU-ACC, GPU-OMP, with/without MPI).

.claude/rules/parameter-system.md

@@ -1,7 +1,7 @@
 # Parameter System
 
 ## Overview
-MFC has ~3,400 simulation parameters defined in Python and read by Fortran via namelist files.
+MFC's simulation parameters are defined in Python and read by Fortran via namelist files.
 
 ## Parameter Flow: Python → Fortran
 
@@ -23,20 +23,25 @@ MFC has ~3,400 simulation parameters defined in Python and read by Fortran via n
    - Reads `&user_inputs` namelist
    - Each parameter must be declared in the namelist statement
 
-## Adding a New Parameter (4-location checklist)
+## Adding a New Parameter (2-location checklist)
 
-YOU MUST update the first 3 locations. Missing any causes silent failures or compile errors.
-Location 4 is required only if the parameter has physics constraints.
+Fortran declarations and namelist bindings are now auto-generated from definitions.py
+at CMake configure time — no manual Fortran edits needed for simple scalar parameters.
 
-1. **`toolchain/mfc/params/definitions.py`**: Add parameter with type, default, constraints
-2. **`src/*/m_global_parameters.fpp`**: Declare the Fortran variable in the relevant
-   target(s). If the param is used by simulation only, add it there. If shared, add to
-   all three targets' m_global_parameters.fpp.
-3. **`src/*/m_start_up.fpp`**: Add to the Fortran `namelist` declaration in the relevant
-   target(s).
-4. **`toolchain/mfc/case_validator.py`**: Add validation rules if the parameter has
+1. **`toolchain/mfc/params/definitions.py`**: Add parameter with `_r()` (type, default,
+   constraints) AND add it to `NAMELIST_VARS` via `_nv()` for the relevant target(s).
+   After editing, re-run cmake (or `./mfc.sh build`) to regenerate the Fortran includes.
+2. **`toolchain/mfc/case_validator.py`**: Add validation rules if the parameter has
    physics constraints. Include `PHYSICS_DOCS` entry with title, category, explanation.
 
+**Exceptions — still require manual Fortran edits:**
+- Array variables (e.g. `logical, dimension(num_fluids_max)`) → declare in `src/*/m_global_parameters.fpp`
+- Derived-type members (`fluid_pp%attr`, `patch_icpp(i)%attr`) → declare in the relevant derived type
+- Case-optimization parameters → add to `CASE_OPT_PARAMS` and the `#:else` block in `src/simulation/m_global_parameters.fpp`.
+  Gotcha: under `--case-optimization` these are baked into the binary and dropped from the simulation namelist
+  (`case_dicts.py` filters them), so changing one needs a *rebuild*, not just a case edit — and building without
+  the flag makes them read from `.inp` again.
+
 ## Case Files
 - Case files are Python scripts (`.py`) that define a dict of parameters
 - Validated with `./mfc.sh validate case.py`
@@ -45,15 +50,24 @@ Location 4 is required only if the parameter has physics constraints.
 - Search parameters with `./mfc.sh params <query>`
 
 ## Fortran-Side Runtime Validation
-Each target has `m_checker*.fpp` files (e.g., `src/simulation/m_checker.fpp`,
-`src/common/m_checker_common.fpp`) containing runtime parameter validation using
-`@:PROHIBIT(condition, message)`. When adding parameters with physics constraints,
-add Fortran-side checks here in addition to `case_validator.py`.
+Runtime parameter validation uses `@:PROHIBIT(condition, message)`. Put a check where it runs:
+- **Shared across all three targets** → `src/common/m_checker_common.fpp` (`s_check_inputs_common`,
+  with `#ifndef MFC_*` gates for target-specific exclusions). This holds most checks.
+- **Simulation-only** → `src/simulation/m_checker.fpp` (WENO/MUSCL/IGR/time-stepping/compiler checks).
+- **Pre/post-only** → `src/{pre,post}_process/m_checker.fpp`. Note: their `s_check_inputs` are
+  currently empty — that's the right place for a pre/post-only constraint, not `m_checker_common.fpp`.
+
+Add Fortran-side checks in addition to `case_validator.py`.
 
 ## Analytical Initial Conditions
 String expressions in parameters become Fortran code via `case.py.__get_analytic_ic_fpp()`.
 These are compiled into the binary, so syntax errors cause build failures, not runtime errors.
 
+Gotcha: each IC variable (`alpha_rho`, `vel`, `pres`, `alpha`, `Y`, `Bx`...) maps to an `eqn_idx%…`
+expression in `QPVF_IDX_VARS` (`case.py`). Adding a conserved variable that patches can set means
+updating that map *and* the Fortran `eqn_idx` builder to agree — a mismatch is a silent wrong-index, not
+an error. (This is also why `Bx`/`By`/`Bz` use `eqn_idx%B%end-1/%end`, to stay valid in 1D/2D.)
+
 Available variables in analytical IC expressions:
 - `x`, `y`, `z` — cell-center coordinates (mapped to `x_cc(i)`, `y_cc(j)`, `z_cc(k)`)
 - `xc`, `yc`, `zc` — patch centroid coordinates

.github/file-filter.yml

@@ -40,6 +40,3 @@ checkall: &checkall
   - *tests
   - *scripts
   - *yml
-
-cases_py:
-  - 'toolchain/mfc/test/cases.py'

.github/pull_request_template.md

@@ -2,22 +2,24 @@
 
 Summarize your changes and the motivation behind them.
 
-Fixes #(issue)
+Closes #(issue number).
 
-### Type of change
+### Type of change (delete unused ones)
 
-- [ ] Bug fix
-- [ ] New feature
-- [ ] Refactor
-- [ ] Documentation
-- [ ] Other: _describe_
+- Bug fix
+- New feature
+- Refactor
+- Documentation
+- Other (describe)
 
 ## Testing
 
 How did you test your changes?
 
 ## Checklist
 
+__Check these like this `[x]` to indicate which of the below applies.__
+
 - [ ] I added or updated tests for new behavior
 - [ ] I updated documentation if user-facing behavior changed
 
@@ -33,10 +35,6 @@ See the [developer guide](https://mflowcode.github.io/documentation/contributing
 
 ## AI code reviews
 
-Reviews are not triggered automatically. To request a review, comment on the PR:
-- `@coderabbitai review` — incremental review (new changes only)
-- `@coderabbitai full review` — full review from scratch
-- `/review` — Qodo review
-- `/improve` — Qodo code suggestions
+Reviews are not retriggered automatically. To request a review, comment on the PR:
 - `@claude full review` — Claude full review (also triggers on PR open/reopen/ready)
-- Add label `claude-full-review` — Claude full review via label
+- Or add label `claude-full-review` — Claude full review via label

.github/scripts/check_coverage_map_health.py

@@ -0,0 +1,42 @@
+"""Fail loudly if the committed coverage map is stale or under-covers. Used by coverage-health.yml."""
+import datetime
+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.cases import list_cases  # noqa: E402  (returns the current test list)
+
+MAX_AGE_DAYS = 10
+MIN_FRACTION = 0.80
+
+entries, meta = load_map(COVERAGE_MAP_PATH)
+if entries is None:
+    sys.exit("Coverage map missing or corrupt.")
+# Compute each current test's coverage key. Loading a case executes its case
+# file; some (e.g. chemistry examples) import optional deps like cantera that are
+# not installed in this lightweight job. Skip any case that fails to load instead
+# of crashing — map_health measures the fraction of *loadable* current tests that
+# are mapped, so a smaller current_keys cannot produce a false "stale" result.
+current_keys = set()
+unloadable = []
+for b in list_cases():
+    try:
+        current_keys.add(b.to_case().coverage_key())
+    except Exception as exc:  # noqa: BLE001 — a case file that won't import must not crash the health check
+        last_line = (str(exc).strip().splitlines() or ["(no message)"])[-1][:140]
+        unloadable.append((getattr(b, "trace", repr(b)), last_line))
+if unloadable:
+    print(f"Note: {len(unloadable)} case(s) could not be loaded in this lightweight job (excluded from the check):")
+    for trace, err in unloadable[:15]:
+        print(f"  - {trace}: {err}")
+ok, msg = map_health(
+    meta=meta,
+    current_keys=current_keys,
+    mapped_keys=set(entries),
+    now=datetime.datetime.now(datetime.timezone.utc).isoformat(),
+    max_age_days=MAX_AGE_DAYS,
+    min_fraction=MIN_FRACTION,
+)
+print(msg)
+sys.exit(0 if ok else 1)

.github/workflows/common/coverage-refresh.sh

@@ -0,0 +1,7 @@
+#!/bin/bash
+set -e
+NJOBS="${SLURM_CPUS_ON_NODE:-24}"; [ "$NJOBS" -gt 64 ] && NJOBS=64
+./mfc.sh clean
+source .github/scripts/retry-build.sh
+retry_build ./mfc.sh build --gcov -j 8
+./mfc.sh test --build-coverage-map --gcov -j "$NJOBS"

.github/workflows/common/rebuild-cache.sh

@@ -57,10 +57,14 @@ if [ -n "${job_shard:-}" ]; then
     shard_opts="--shard $job_shard"
 fi
 
-# Only prune tests on PRs; master pushes must run the full suite.
-prune_flag=""
+# Coverage-based test selection ENFORCED on PRs: runs only tests whose recorded coverage
+# overlaps the PR's changed files (conservative ladder; non-.fpp and uncovered-.fpp changes
+# fall back to run-all). Pushes to master run the full suite as a backstop. Changed files
+# come from git detection (self-healing deepen) since the SLURM job doesn't receive the
+# paths-filter list.
+select_opts=""
 if [ "${GITHUB_EVENT_NAME:-}" = "pull_request" ]; then
-    prune_flag="--only-changes"
+    select_opts="--select-enforce"
 fi
 
-./mfc.sh test -v --max-attempts 3 --no-build $prune_flag -a -j $n_test_threads $rdma_opts $device_opts $build_opts $shard_opts -- -c $job_cluster
+./mfc.sh test -v --max-attempts 3 --no-build $select_opts -a -j $n_test_threads $rdma_opts $device_opts $build_opts $shard_opts -- -c $job_cluster