Skip to content

Commit 2ace5f1

Browse files
Merge branch 'local-aware-ibm' of github.com:danieljvickers/MFC into local-aware-ibm
2 parents cba5e32 + b41e831 commit 2ace5f1

27 files changed

Lines changed: 1354 additions & 1957 deletions

.claude/rules/common-pitfalls.md

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
- Grid dimensions: `m`, `n`, `p` (cells in x, y, z). 1D: n=p=0. 2D: p=0.
55
- Interior domain: `0:m`, `0:n`, `0:p`
66
- Buffer/ghost region: `-buff_size:m+buff_size` (similar for n, p in multi-D)
7-
- `buff_size` depends on WENO order and features (typically `2*weno_polyn + 2`)
7+
- `buff_size` is **not** a single formula: it's set per reconstruction scheme (WENO/MUSCL/IGR) in
8+
`s_configure_coordinate_bounds` (`m_helper_basic.fpp`) and floored higher for Lagrange bubbles and IB.
9+
Read that routine for the current value rather than assuming one.
810
- Domain bounds: `idwint(1:3)` (interior `0:m`), `idwbuff(1:3)` (with ghost cells)
911
- Cell-center coords: `x_cc(-buff_size:m+buff_size)`, `y_cc(...)`, `z_cc(...)`
1012
- Cell-boundary coords: `x_cb(-1-buff_size:m+buff_size)`
@@ -38,8 +40,8 @@
3840
- Boundary condition symmetry requirements must be maintained
3941

4042
## Compiler-Specific Issues
41-
- CI-gated compilers (must always pass): gfortran, nvfortran, Cray ftn, and Intel ifx
42-
- AMD flang is additionally supported for `--gpu mp` builds but not in the CI matrix
43+
- See the compiler-backend matrix in `.claude/rules/gpu-and-mpi.md` for which compilers
44+
are CI-gated and which backends each supports.
4345
- Each compiler has different strictness levels and warning behavior
4446
- Fypp macros must expand correctly for both GPU and CPU builds
4547

@@ -54,12 +56,8 @@
5456
- Do not regenerate ALL golden files unless you understand every output change
5557

5658
## PR Checklist
57-
Before submitting a PR:
58-
- [ ] `./mfc.sh format -j 8` (auto-format)
59-
- [ ] `./mfc.sh precheck -j 8` (5 CI lint checks)
60-
- [ ] `./mfc.sh build -j 8` (compiles)
61-
- [ ] `./mfc.sh test --only <relevant> -j 8` (tests pass)
59+
The base loop (format → precheck → build → test → one logical commit) is the
60+
Development Workflow Contract in `CLAUDE.md`. Beyond it, watch for:
6261
- [ ] If adding parameters: definitions.py (_r + _nv) updated; cmake reconfigured; case_validator.py if constraints
6362
- [ ] If modifying `src/common/`: all three targets tested
64-
- [ ] If changing output: golden files regenerated for affected tests
65-
- [ ] One logical change per commit
63+
- [ ] If changing output: golden files regenerated for affected tests (`./mfc.sh test --generate --only <tests>`)

.claude/rules/fortran-conventions.md

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,7 @@ Every Fortran module follows this pattern:
1515
- Finalization subroutine: `s_finalize_<feature>_module`
1616

1717
## Naming
18-
- Modules: `m_<feature>`
19-
- Public subroutines: `s_<verb>_<noun>`
20-
- Public functions: `f_<verb>_<noun>`
21-
- Private/local variables: no prefix required
22-
- Constants: descriptive names, not ALL_CAPS
18+
See "Naming Conventions" in `CLAUDE.md`.
2319

2420
## Forbidden Patterns
2521

@@ -57,10 +53,8 @@ Enforced by convention/code review (not automated):
5753
- Fortran-side runtime validation also exists in `m_checker*.fpp` files using `@:PROHIBIT`
5854

5955
## Precision Types
60-
- `wp` (working precision): used for computation. Double by default.
61-
- `stp` (storage precision): used for field data arrays and I/O. Double by default.
62-
- In single-precision mode (`--single`): both become single.
63-
- In mixed-precision mode (`--mixed`): wp=double, stp=half.
56+
`wp`/`stp` are defined in `CLAUDE.md` (`wp` = computation, `stp` = field-data storage + I/O). Detail:
57+
- Modes: default both double; `--single` → both single; `--mixed` → wp=double, stp=half.
6458
- MPI type matching: `mpi_p` must match `wp`, `mpi_io_p` must match `stp`.
6559
- Always use generic intrinsics: `sqrt` not `dsqrt`, `abs` not `dabs`.
6660
- Cast with `real(..., wp)` or `real(..., stp)`, never `dble(...)`.

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

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,26 +23,17 @@ compiles to either OpenACC or OpenMP target offload depending on the build flag:
2323

2424
### Key GPU Macros (always use the `GPU_*` prefix)
2525

26-
Inline macros (use `$:` prefix):
27-
- `$:GPU_PARALLEL_LOOP(collapse=N, private=[...], reduction=[...], reductionOp='+')`
28-
Parallel loop over GPU threads. Most common GPU macro.
29-
- `$:END_GPU_PARALLEL_LOOP()` — Required closing for GPU_PARALLEL_LOOP.
30-
- `$:GPU_LOOP(collapse=N, ...)` — Inner loop within a GPU parallel region.
31-
- `$:GPU_ENTER_DATA(create=[...])` — Allocate device memory (unscoped).
32-
- `$:GPU_EXIT_DATA(delete=[...])` — Free device memory.
33-
- `$:GPU_UPDATE(host=[...])` — Copy device → host (before MPI send).
34-
- `$:GPU_UPDATE(device=[...])` — Copy host → device (after MPI receive).
35-
- `$:GPU_ROUTINE(parallelism='[seq]')` — Mark routine for device compilation.
36-
- `$:GPU_DECLARE(create=[...])` — Declare device-resident data.
37-
- `$:GPU_ATOMIC(atomic='update')` — Atomic operation on device.
38-
- `$:GPU_WAIT()` — Synchronization barrier.
39-
40-
Block macros (use `#:call`/`#:endcall`):
41-
- `GPU_PARALLEL(...)` — GPU parallel region (used for scalar reductions like `maxval`/`minval`).
42-
- `GPU_DATA(copy=..., create=..., ...)` — Scoped data region.
43-
- `GPU_HOST_DATA(use_device_addr=[...])` — Host code with device pointers.
44-
45-
Typical GPU loop pattern (used 750+ times in the codebase):
26+
Full set with signatures in `parallel_macros.fpp`. The ones you reach for most:
27+
- `$:GPU_PARALLEL_LOOP(collapse=N, private=[...], reduction=[...], reductionOp='+')`
28+
+ `$:END_GPU_PARALLEL_LOOP()` — parallel spatial loop; by far the most common (see pattern below).
29+
- `$:GPU_LOOP(collapse=N, ...)` — inner loop *within* a parallel region.
30+
- `$:GPU_UPDATE(host=[...])` / `$:GPU_UPDATE(device=[...])` — device↔host copies (around MPI; see below).
31+
- `#:call GPU_PARALLEL(...)` — block region for scalar reductions (`maxval`/`minval`).
32+
33+
Others in `parallel_macros.fpp`: `GPU_ENTER_DATA`/`GPU_EXIT_DATA`, `GPU_DECLARE`, `GPU_ROUTINE`,
34+
`GPU_ATOMIC`, `GPU_WAIT`, and the block macros `GPU_DATA`, `GPU_HOST_DATA`.
35+
36+
Typical GPU loop pattern (the dominant spatial-loop idiom):
4637
```
4738
$:GPU_PARALLEL_LOOP(private='[i,j,k,l]', collapse=3)
4839
do l = idwbuff(3)%beg, idwbuff(3)%end
@@ -108,16 +99,10 @@ Use `#ifdef` for feature, target, compiler, and library gating:
10899
- `MFC_POST_PROCESS` — Only in post_process builds
109100

110101
### Compiler gating (for compiler-specific workarounds)
111-
- `_CRAYFTN` — Cray Fortran compiler
112-
- `__NVCOMPILER_GPU_UNIFIED_MEM` — NVIDIA unified memory (GH-200 / `--unified`)
113-
- `__PGI` — Legacy PGI/NVIDIA compiler
114-
- `__INTEL_COMPILER` — Intel compiler
115-
- `FRONTIER_UNIFIED` — Frontier HPC unified memory
116-
117-
### Library-specific code
118-
- FFTW (`m_fftw.fpp`) uses heavy `#ifdef` gating for `MFC_GPU` and `__PGI`
119-
- CUDA Fortran (`cudafor` module) is gated behind `__NVCOMPILER_GPU_UNIFIED_MEM`
120-
- SILO/HDF5 interfaces may have conditional paths
102+
Compiler/feature macros: `_CRAYFTN`, `__NVCOMPILER_GPU_UNIFIED_MEM` (NVIDIA unified mem, GH-200 /
103+
`--unified`), `__PGI` (legacy PGI/NVIDIA), `__INTEL_COMPILER`, `FRONTIER_UNIFIED`. Library code is
104+
similarly gated (FFTW in `m_fftw.fpp` on `MFC_GPU`/`__PGI`; CUDA Fortran `cudafor` on
105+
`__NVCOMPILER_GPU_UNIFIED_MEM`; SILO/HDF5 paths). Grep the relevant file for exact usage.
121106

122107
When adding new `#ifdef` blocks, always provide an `#else` or `#endif` path so
123108
the code compiles in all configurations (CPU-only, GPU-ACC, GPU-OMP, with/without MPI).

.claude/rules/parameter-system.md

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Parameter System
22

33
## Overview
4-
MFC has ~3,400 simulation parameters defined in Python and read by Fortran via namelist files.
4+
MFC's simulation parameters are defined in Python and read by Fortran via namelist files.
55

66
## Parameter Flow: Python → Fortran
77

@@ -37,7 +37,10 @@ at CMake configure time — no manual Fortran edits needed for simple scalar par
3737
**Exceptions — still require manual Fortran edits:**
3838
- Array variables (e.g. `logical, dimension(num_fluids_max)`) → declare in `src/*/m_global_parameters.fpp`
3939
- Derived-type members (`fluid_pp%attr`, `patch_icpp(i)%attr`) → declare in the relevant derived type
40-
- Case-optimization parameters → add to `CASE_OPT_PARAMS` and the `#:else` block in `src/simulation/m_global_parameters.fpp`
40+
- Case-optimization parameters → add to `CASE_OPT_PARAMS` and the `#:else` block in `src/simulation/m_global_parameters.fpp`.
41+
Gotcha: under `--case-optimization` these are baked into the binary and dropped from the simulation namelist
42+
(`case_dicts.py` filters them), so changing one needs a *rebuild*, not just a case edit — and building without
43+
the flag makes them read from `.inp` again.
4144

4245
## Case Files
4346
- Case files are Python scripts (`.py`) that define a dict of parameters
@@ -47,15 +50,24 @@ at CMake configure time — no manual Fortran edits needed for simple scalar par
4750
- Search parameters with `./mfc.sh params <query>`
4851

4952
## Fortran-Side Runtime Validation
50-
Each target has `m_checker*.fpp` files (e.g., `src/simulation/m_checker.fpp`,
51-
`src/common/m_checker_common.fpp`) containing runtime parameter validation using
52-
`@:PROHIBIT(condition, message)`. When adding parameters with physics constraints,
53-
add Fortran-side checks here in addition to `case_validator.py`.
53+
Runtime parameter validation uses `@:PROHIBIT(condition, message)`. Put a check where it runs:
54+
- **Shared across all three targets**`src/common/m_checker_common.fpp` (`s_check_inputs_common`,
55+
with `#ifndef MFC_*` gates for target-specific exclusions). This holds most checks.
56+
- **Simulation-only**`src/simulation/m_checker.fpp` (WENO/MUSCL/IGR/time-stepping/compiler checks).
57+
- **Pre/post-only**`src/{pre,post}_process/m_checker.fpp`. Note: their `s_check_inputs` are
58+
currently empty — that's the right place for a pre/post-only constraint, not `m_checker_common.fpp`.
59+
60+
Add Fortran-side checks in addition to `case_validator.py`.
5461

5562
## Analytical Initial Conditions
5663
String expressions in parameters become Fortran code via `case.py.__get_analytic_ic_fpp()`.
5764
These are compiled into the binary, so syntax errors cause build failures, not runtime errors.
5865

66+
Gotcha: each IC variable (`alpha_rho`, `vel`, `pres`, `alpha`, `Y`, `Bx`...) maps to an `eqn_idx%…`
67+
expression in `QPVF_IDX_VARS` (`case.py`). Adding a conserved variable that patches can set means
68+
updating that map *and* the Fortran `eqn_idx` builder to agree — a mismatch is a silent wrong-index, not
69+
an error. (This is also why `Bx`/`By`/`Bz` use `eqn_idx%B%end-1/%end`, to stay valid in 1D/2D.)
70+
5971
Available variables in analytical IC expressions:
6072
- `x`, `y`, `z` — cell-center coordinates (mapped to `x_cc(i)`, `y_cc(j)`, `z_cc(k)`)
6173
- `xc`, `yc`, `zc` — patch centroid coordinates

.github/file-filter.yml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,3 @@ checkall: &checkall
4040
- *tests
4141
- *scripts
4242
- *yml
43-
44-
cases_py:
45-
- 'toolchain/mfc/test/cases.py'
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
"""Fail loudly if the committed coverage map is stale or under-covers. Used by coverage-health.yml."""
2+
import datetime
3+
import sys
4+
from pathlib import Path
5+
6+
sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "toolchain"))
7+
from mfc.test.coverage import COVERAGE_MAP_PATH, load_map, map_health # noqa: E402
8+
from mfc.test.cases import list_cases # noqa: E402 (returns the current test list)
9+
10+
MAX_AGE_DAYS = 10
11+
MIN_FRACTION = 0.80
12+
13+
entries, meta = load_map(COVERAGE_MAP_PATH)
14+
if entries is None:
15+
sys.exit("Coverage map missing or corrupt.")
16+
current_keys = {b.to_case().coverage_key() for b in list_cases()}
17+
ok, msg = map_health(
18+
meta=meta,
19+
current_keys=current_keys,
20+
mapped_keys=set(entries),
21+
now=datetime.datetime.now(datetime.timezone.utc).isoformat(),
22+
max_age_days=MAX_AGE_DAYS,
23+
min_fraction=MIN_FRACTION,
24+
)
25+
print(msg)
26+
sys.exit(0 if ok else 1)
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#!/bin/bash
2+
set -e
3+
NJOBS="${SLURM_CPUS_ON_NODE:-24}"; [ "$NJOBS" -gt 64 ] && NJOBS=64
4+
./mfc.sh clean
5+
source .github/scripts/retry-build.sh
6+
retry_build ./mfc.sh build --gcov -j 8
7+
./mfc.sh test --build-coverage-map --gcov -j "$NJOBS"

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

Lines changed: 0 additions & 23 deletions
This file was deleted.

.github/workflows/common/test.sh

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,12 @@ if [ -n "${job_shard:-}" ]; then
5757
shard_opts="--shard $job_shard"
5858
fi
5959

60-
# Only prune tests on PRs; master pushes must run the full suite.
61-
prune_flag=""
60+
# Coverage-based test selection in SHADOW mode on PRs: prints what it WOULD select but the
61+
# full suite still runs (no --select-enforce). Changed files come from git detection
62+
# (self-healing deepen) since the SLURM job doesn't receive the paths-filter list.
63+
select_opts=""
6264
if [ "${GITHUB_EVENT_NAME:-}" = "pull_request" ]; then
63-
prune_flag="--only-changes"
65+
select_opts="--only-changes"
6466
fi
6567

66-
./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
68+
./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
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# .github/workflows/coverage-health.yml
2+
name: 'Coverage Map Health'
3+
on:
4+
schedule:
5+
- cron: '0 7 * * *' # daily; loud if the refresh stopped working
6+
workflow_dispatch:
7+
jobs:
8+
health:
9+
if: github.repository == 'MFlowCode/MFC'
10+
runs-on: ubuntu-latest
11+
steps:
12+
- uses: actions/checkout@v4
13+
- uses: actions/setup-python@v5
14+
with: { python-version: '3.12' }
15+
- name: Initialize MFC
16+
run: ./mfc.sh init
17+
- name: Check coverage map freshness
18+
run: build/venv/bin/python3 .github/scripts/check_coverage_map_health.py

0 commit comments

Comments
 (0)