-
Notifications
You must be signed in to change notification settings - Fork 132
Add CLAUDE.md and .claude/rules/ for Claude Code guidance #1255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+487
−1
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
cb58059
Add CLAUDE.md and .claude/rules/ for Claude Code guidance
sbryngelson 7895c6f
Update copilot-instructions.md to mention OpenMP target offload
sbryngelson 041cfa6
Fix parameter checklist and validation attribution
sbryngelson 20da91f
Fix mixed-precision stp description: half, not single
sbryngelson 5427de1
Fix Fypp macro syntax and precheck coverage claims
sbryngelson 3286cfa
Fix documentation accuracy: checklist counts, phantom macro, missing …
sbryngelson 7104b11
Add field indexing, ghost cells, test system, analytical IC docs
sbryngelson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| # Common Pitfalls | ||
|
|
||
| ## Array Bounds & Ghost Cells | ||
| - 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`) | ||
| - 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)` | ||
| - Riemann solver indexing: left state at `j`, right state at `j+1` | ||
| - Off-by-one errors in ghost cell regions are a common source of bugs | ||
|
|
||
| ## Field Variable Indexing | ||
| - Conserved variables: `q_cons_vf(1:sys_size)`. Primitive: `q_prim_vf(1:sys_size)`. | ||
| - Index ranges depend on `model_eqns` and enabled features (set in `m_global_parameters.fpp`): | ||
| - `cont_idx` — continuity (partial densities, one per fluid) | ||
| - `mom_idx` — momentum components | ||
| - `E_idx` — total energy (scalar) | ||
| - `adv_idx` — volume fractions (advection equations) | ||
| - `bub_idx`, `stress_idx`, `xi_idx`, `species_idx`, `B_idx`, `c_idx` — optional | ||
| - Shorthand scalars: `momxb`/`momxe`, `contxb`/`contxe`, `advxb`/`advxe`, etc. | ||
| - `sys_size` = total number of conserved variables (computed at startup) | ||
| - Changing `model_eqns` or enabling features changes ALL index positions | ||
|
|
||
| ## Blast Radius | ||
| - `src/common/` is shared by ALL three executables (pre_process, simulation, post_process) | ||
| - Any change to common/ requires testing all three targets | ||
| - Public subroutine signature changes affect all callers across all targets | ||
| - Parameter default changes affect all existing case files | ||
|
|
||
| ## Physics Consistency | ||
| - Pressure formula MUST match `model_eqns` setting | ||
| - Model-specific conservative ↔ primitive conversion paths exist | ||
| - Volume fractions must sum to 1.0 | ||
| - Boundary condition symmetry requirements must be maintained | ||
|
|
||
| ## Compiler-Specific Issues | ||
| - Code must compile on gfortran, nvfortran, Cray ftn, and Intel ifx | ||
| - Each compiler has different strictness levels and warning behavior | ||
| - Fypp macros must expand correctly for both GPU and CPU builds | ||
| - GPU builds only work with nvfortran, Cray ftn, and AMD flang | ||
|
|
||
| ## Test System | ||
| - Tests are generated **programmatically** in `toolchain/mfc/test/cases.py`, not standalone files | ||
| - Each test is a parameter modification on top of `BASE_CFG` defaults | ||
| - Test UUID = CRC32 hash of the test's trace string; `./mfc.sh test -l` lists all | ||
| - To add a test: modify `cases.py` using `CaseGeneratorStack` push/pop pattern | ||
| - Golden files: `tests/<UUID>/golden.txt` — tolerance-based comparison, not exact match | ||
| - If your change intentionally modifies output, regenerate golden files: | ||
| `./mfc.sh test --generate --only <affected_tests> -j 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 | ||
| - [ ] If modifying `src/common/`: all three targets tested | ||
| - [ ] If changing output: golden files regenerated for affected tests | ||
| - [ ] One logical change per commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| # Fortran Conventions | ||
|
|
||
| ## File Format | ||
| - Source files use `.fpp` extension (Fortran + Fypp preprocessor macros) | ||
| - Fypp preprocesses `.fpp` → `.f90` at build time via CMake | ||
| - Fypp supports conditional compilation, code generation, and regex macros | ||
|
|
||
| ## Module Structure | ||
| Every Fortran module follows this pattern: | ||
| - File: `m_<feature>.fpp` | ||
| - Module: `module m_<feature>` | ||
| - `implicit none` required | ||
| - Explicit `intent(in)`, `intent(out)`, or `intent(inout)` on ALL subroutine/function arguments | ||
| - Initialization subroutine: `s_initialize_<feature>_module` | ||
| - 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 | ||
|
|
||
| ## Forbidden Patterns | ||
|
|
||
| Caught by `./mfc.sh precheck` (source lint step 4/5): | ||
| - `dsqrt`, `dexp`, `dlog`, `dble`, `dabs`, `dcos`, `dsin`, `dtan`, etc. → use generic intrinsics | ||
| - `1.0d0`, `2.5d-3` (Fortran `d` exponent literals) → use `1.0_wp`, `2.5e-3_wp` | ||
| - `double precision` → use `real(wp)` or `real(stp)` | ||
| - `real(8)`, `real(4)` → use `wp` or `stp` kind parameters | ||
| - Raw `!$acc` or `!$omp` directives → use Fypp GPU_* macros from `parallel_macros.fpp` | ||
| - Full list of forbidden patterns: `toolchain/bootstrap/precheck.sh` | ||
|
|
||
| Enforced by convention/code review (not automated): | ||
| - `goto`, `COMMON` blocks, global `save` variables | ||
| - `stop`, `error stop` → use `call s_mpi_abort()` or `@:PROHIBIT()`/`@:ASSERT()` | ||
|
|
||
| ## Error Checking Macros (from macros.fpp) | ||
| - `@:PROHIBIT(condition, message)` — Runtime constraint check; aborts with file/line info | ||
| - `@:ASSERT(predicate, message)` — Invariant assertion; aborts if predicate is false | ||
| - `@:LOG(expr)` — Debug logging, active only in `MFC_DEBUG` builds | ||
| - 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. | ||
| - 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(...)`. | ||
|
|
||
| Key derived types (`m_derived_types.fpp`): | ||
| - `scalar_field` — `real(stp), pointer :: sf(:,:,:)`. Uses `stp`, NOT `wp`. | ||
| - `vector_field` — allocatable array of `scalar_field` components. | ||
| - New field arrays MUST use `stp` for storage precision consistency. | ||
|
|
||
| ## Size Guidelines (soft) | ||
| - Subroutine: ≤500 lines | ||
| - Helper routine: ≤150 lines | ||
| - Function: ≤100 lines | ||
| - File: ≤1000 lines | ||
| - Arguments: ≤6 preferred |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| # GPU and MPI Patterns | ||
|
|
||
| ## GPU Offloading Architecture | ||
|
|
||
| Only `src/simulation/` is GPU-accelerated. Pre/post_process run on CPU only. | ||
|
|
||
| MFC uses a **backend-agnostic GPU abstraction** via Fypp macros. The same source code | ||
| compiles to either OpenACC or OpenMP target offload depending on the build flag: | ||
|
|
||
| - `./mfc.sh build --gpu acc` → OpenACC backend (NVIDIA nvfortran, Cray ftn) | ||
| - `./mfc.sh build --gpu mp` → OpenMP target offload backend (Cray ftn, AMD flang) | ||
| - `./mfc.sh build` (no --gpu) → CPU-only, GPU macros expand to plain Fortran | ||
|
|
||
| ### Macro Layers (in src/common/include/) | ||
| - `parallel_macros.fpp` — **Use these.** Generic `GPU_*` macros that dispatch to the | ||
| correct backend based on `MFC_OpenACC` / `MFC_OpenMP` compile definitions. | ||
| - `acc_macros.fpp` — OpenACC-specific `ACC_*` implementations (do not call directly) | ||
| - `omp_macros.fpp` — OpenMP target offload `OMP_*` implementations (do not call directly) | ||
| - OMP macros generate **compiler-specific** directives: NVIDIA uses `target teams loop`, | ||
| Cray uses `target teams distribute parallel do simd`, AMD uses | ||
| `target teams distribute parallel do` | ||
| - `shared_parallel_macros.fpp` — Shared helpers (collapse, private, reduction generators) | ||
|
|
||
| ### 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 wrapping a code block. | ||
| - `GPU_DATA(copy=..., create=..., ...)` — Scoped data region. | ||
| - `GPU_HOST_DATA(use_device_addr=[...])` — Host code with device pointers. | ||
|
|
||
| Block macro usage: | ||
| ``` | ||
| #:call GPU_PARALLEL(copyin='[var1]', copyout='[var2]') | ||
| $:GPU_LOOP(collapse=N) | ||
| do k = 0, n; do j = 0, m | ||
| ! loop body | ||
| end do; end do | ||
| #:endcall GPU_PARALLEL | ||
| ``` | ||
|
|
||
| NEVER write raw `!$acc` or `!$omp` directives. Always use `GPU_*` Fypp macros. | ||
| The precheck source lint will catch raw directives and fail. | ||
|
|
||
| ### Memory Management Macros (from macros.fpp) | ||
| - `@:ALLOCATE(var1, var2, ...)` — Fortran allocate + `GPU_ENTER_DATA(create=...)` | ||
| - `@:DEALLOCATE(var1, var2, ...)` — `GPU_EXIT_DATA(delete=...)` + Fortran deallocate | ||
| - `@:PREFER_GPU(var1, var2, ...)` — NVIDIA unified memory page placement hint | ||
| - Every `@:ALLOCATE` MUST have a matching `@:DEALLOCATE` in finalization | ||
| - Conditional allocation MUST have conditional deallocation | ||
|
|
||
| ### GPU Field Setup (Cray-specific, from macros.fpp) | ||
| - `@:ACC_SETUP_VFs(...)` / `@:ACC_SETUP_SFs(...)` — GPU pointer setup for vector/scalar fields | ||
| - These compile only for Cray (`_CRAYFTN`); other compilers skip them | ||
|
|
||
| ### Compiler-Backend Matrix | ||
| | Compiler | `--gpu acc` (OpenACC) | `--gpu mp` (OpenMP) | CPU-only | | ||
| |-----------------|----------------------|---------------------|----------| | ||
| | GNU gfortran | No | No | Yes | | ||
| | NVIDIA nvfortran| Yes (primary) | Yes | Yes | | ||
| | Cray ftn (CCE) | Yes | Yes (primary) | Yes | | ||
| | Intel ifx | No | No | Yes | | ||
| | AMD flang | No | Yes | Yes | | ||
|
|
||
| ## Preprocessor Defines (`#ifdef` / `#ifndef`) | ||
|
|
||
| Raw `#ifdef` / `#ifndef` preprocessor guards are **normal and expected** in MFC. | ||
| They are NOT the same as raw `!$acc`/`!$omp` pragmas (which are forbidden). | ||
|
|
||
| Use `#ifdef` for feature, target, compiler, and library gating: | ||
|
|
||
| ### Feature gating | ||
| - `MFC_MPI` — MPI-enabled build (`--mpi` flag, default ON) | ||
| - `MFC_OpenACC` — OpenACC GPU backend (`--gpu acc`) | ||
| - `MFC_OpenMP` — OpenMP target offload backend (`--gpu mp`) | ||
| - `MFC_GPU` — Any GPU build (either OpenACC or OpenMP) | ||
| - `MFC_DEBUG` — Debug build (`--debug`) | ||
| - `MFC_SINGLE_PRECISION` — Single-precision mode (`--single`) | ||
| - `MFC_MIXED_PRECISION` — Mixed-precision mode (`--mixed`) | ||
|
|
||
| ### Target gating (for code in `src/common/` shared across executables) | ||
| - `MFC_PRE_PROCESS` — Only in pre_process builds | ||
| - `MFC_SIMULATION` — Only in simulation builds | ||
| - `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 | ||
|
|
||
| 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). | ||
|
|
||
| ## MPI | ||
|
|
||
| ### Halo Exchange | ||
| - Pack/unpack offset calculations are error-prone — verify carefully | ||
| - Buffer sizing depends on dimensionality and QBMM state | ||
| - GPU coherence: always `GPU_UPDATE(host=...)` before MPI send, | ||
| `GPU_UPDATE(device=...)` after MPI receive | ||
|
|
||
| ### Error Handling | ||
| - Use `call s_mpi_abort()` for fatal errors, never `stop` or `error stop` | ||
| - MPI must be finalized before program exit | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # Parameter System | ||
|
|
||
| ## Overview | ||
| MFC has ~3,400 simulation parameters defined in Python and read by Fortran via namelist files. | ||
|
|
||
| ## Parameter Flow: Python → Fortran | ||
|
|
||
| 1. **Definition**: `toolchain/mfc/params/definitions.py` — source of truth | ||
| - Parameters are indexed families: `patch_icpp(i)%attr`, `fluid_pp(i)%attr`, etc. | ||
| - Each has type, default, constraints, and tags | ||
|
|
||
| 2. **Validation** (two layers): | ||
| - `toolchain/mfc/case.py` / `toolchain/mfc/params/registry.py` — JSON schema validation | ||
| via fastjsonschema (type checking, defaults) | ||
| - `toolchain/mfc/case_validator.py` — Physics constraint checking | ||
| (e.g., volume fractions sum to 1, dependency validation) | ||
|
|
||
| 3. **Input Generation**: `toolchain/mfc/run/input.py` | ||
| - Python case dict → Fortran namelist `.inp` file | ||
| - Format: `&user_inputs` ... `&end/` | ||
|
|
||
| 4. **Fortran Reading**: `src/*/m_start_up.fpp` | ||
| - Reads `&user_inputs` namelist | ||
| - Each parameter must be declared in the namelist statement | ||
|
|
||
| ## Adding a New Parameter (4-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. | ||
|
|
||
| 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 | ||
| physics constraints. Include `PHYSICS_DOCS` entry with title, category, explanation. | ||
|
|
||
| ## Case Files | ||
| - Case files are Python scripts (`.py`) that define a dict of parameters | ||
| - Validated with `./mfc.sh validate case.py` | ||
| - Examples in `examples/` directory | ||
| - Create new cases with `./mfc.sh new <name>` | ||
| - 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`. | ||
|
|
||
| ## 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. | ||
|
|
||
| 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 | ||
| - `lx`, `ly`, `lz` — patch lengths | ||
| - `r` — patch radius; `eps`, `beta` — vortex parameters | ||
| - `e` — Euler's number (2.71828...) | ||
| - Standard Fortran math intrinsics available: `sin`, `cos`, `exp`, `sqrt`, `abs`, etc. | ||
| - For moving immersed boundaries: `t` (simulation time) is also available | ||
|
|
||
| Example: `'patch_icpp(1)%vel(2)': '(x - xc) * exp(-((x-xc)**2 + (y-yc)**2))'` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.