Skip to content

Commit 3286cfa

Browse files
sbryngelsonclaude
andcommitted
Fix documentation accuracy: checklist counts, phantom macro, missing patterns
- Fix "3-location checklist" heading to "4-location" (parameter-system.md) - Fix "all 3 locations" to "all 4 locations" in PR checklist (common-pitfalls.md) - Remove non-existent $:END_GPU_LOOP() from block macro example (gpu-and-mpi.md) - Add d0 literals, double precision, dcos/dsin/dtan to forbidden patterns list - Reference toolchain/bootstrap/precheck.sh for full forbidden pattern list - Fix Frontier system slug: OpenACC/OpenMP, not just OpenACC - Clarify stp as "field data arrays and I/O", not just "I/O" - Document @:PROHIBIT, @:ASSERT, @:LOG error-checking macros - Document @:PREFER_GPU unified memory macro - Document m_checker*.fpp Fortran-side runtime validation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5427de1 commit 3286cfa

5 files changed

Lines changed: 25 additions & 9 deletions

File tree

.claude/rules/common-pitfalls.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ Before submitting a PR:
3636
- [ ] `./mfc.sh precheck -j 8` (5 CI lint checks)
3737
- [ ] `./mfc.sh build -j 8` (compiles)
3838
- [ ] `./mfc.sh test --only <relevant> -j 8` (tests pass)
39-
- [ ] If adding parameters: all 3 locations updated
39+
- [ ] If adding parameters: all 4 locations updated
4040
- [ ] If modifying `src/common/`: all three targets tested
4141
- [ ] If changing output: golden files regenerated for affected tests
4242
- [ ] One logical change per commit

.claude/rules/fortran-conventions.md

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,26 @@ Every Fortran module follows this pattern:
2424
## Forbidden Patterns
2525

2626
Caught by `./mfc.sh precheck` (source lint step 4/5):
27-
- `dsqrt`, `dexp`, `dlog`, `dble`, `dabs` → use `sqrt`, `exp`, `log`, `real(..., wp)`
27+
- `dsqrt`, `dexp`, `dlog`, `dble`, `dabs`, `dcos`, `dsin`, `dtan`, etc. → use generic intrinsics
28+
- `1.0d0`, `2.5d-3` (Fortran `d` exponent literals) → use `1.0_wp`, `2.5e-3_wp`
29+
- `double precision` → use `real(wp)` or `real(stp)`
2830
- `real(8)`, `real(4)` → use `wp` or `stp` kind parameters
2931
- Raw `!$acc` or `!$omp` directives → use Fypp GPU_* macros from `parallel_macros.fpp`
32+
- Full list of forbidden patterns: `toolchain/bootstrap/precheck.sh`
3033

3134
Enforced by convention/code review (not automated):
3235
- `goto`, `COMMON` blocks, global `save` variables
33-
- `stop`, `error stop` → use `call s_mpi_abort()`
36+
- `stop`, `error stop` → use `call s_mpi_abort()` or `@:PROHIBIT()`/`@:ASSERT()`
37+
38+
## Error Checking Macros (from macros.fpp)
39+
- `@:PROHIBIT(condition, message)` — Runtime constraint check; aborts with file/line info
40+
- `@:ASSERT(predicate, message)` — Invariant assertion; aborts if predicate is false
41+
- `@:LOG(expr)` — Debug logging, active only in `MFC_DEBUG` builds
42+
- Fortran-side runtime validation also exists in `m_checker*.fpp` files using `@:PROHIBIT`
3443

3544
## Precision Types
3645
- `wp` (working precision): used for computation. Double by default.
37-
- `stp` (storage precision): used for I/O. Double by default.
46+
- `stp` (storage precision): used for field data arrays and I/O. Double by default.
3847
- In single-precision mode (`--single`): both become single.
3948
- In mixed-precision mode (`--mixed`): wp=double, stp=half.
4049
- MPI type matching: `mpi_p` must match `wp`, `mpi_io_p` must match `stp`.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ Block macro usage:
4949
do k = 0, n; do j = 0, m
5050
! loop body
5151
end do; end do
52-
$:END_GPU_LOOP()
5352
#:endcall GPU_PARALLEL
5453
```
5554

@@ -59,6 +58,7 @@ The precheck source lint will catch raw directives and fail.
5958
### Memory Management Macros (from macros.fpp)
6059
- `@:ALLOCATE(var1, var2, ...)` — Fortran allocate + `GPU_ENTER_DATA(create=...)`
6160
- `@:DEALLOCATE(var1, var2, ...)``GPU_EXIT_DATA(delete=...)` + Fortran deallocate
61+
- `@:PREFER_GPU(var1, var2, ...)` — NVIDIA unified memory page placement hint
6262
- Every `@:ALLOCATE` MUST have a matching `@:DEALLOCATE` in finalization
6363
- Conditional allocation MUST have conditional deallocation
6464

.claude/rules/parameter-system.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ MFC has ~3,300 simulation parameters defined in Python and read by Fortran via n
2323
- Reads `&user_inputs` namelist
2424
- Each parameter must be declared in the namelist statement
2525

26-
## Adding a New Parameter (3-location checklist)
26+
## Adding a New Parameter (4-location checklist)
2727

2828
YOU MUST update all 4 locations. Missing any causes silent failures or compile errors.
2929

@@ -43,6 +43,12 @@ YOU MUST update all 4 locations. Missing any causes silent failures or compile e
4343
- Create new cases with `./mfc.sh new <name>`
4444
- Search parameters with `./mfc.sh params <query>`
4545

46+
## Fortran-Side Runtime Validation
47+
Each target has `m_checker*.fpp` files (e.g., `src/simulation/m_checker.fpp`,
48+
`src/common/m_checker_common.fpp`) containing runtime parameter validation using
49+
`@:PROHIBIT(condition, message)`. When adding parameters with physics constraints,
50+
add Fortran-side checks here in addition to `case_validator.py`.
51+
4652
## Analytical Initial Conditions
4753
String expressions in parameters become Fortran code via `case.py.__get_analytic_ic_fpp()`.
4854
These are compiled into the binary, so syntax errors cause build failures, not runtime errors.

CLAUDE.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ Supported systems and their slugs (full list in `toolchain/modules`):
7575
| Slug | System | GPU Backend | Example |
7676
|------|--------|-------------|---------|
7777
| `p` | GT Phoenix | OpenACC (nvfortran) | `source ./mfc.sh load -c p -m g` |
78-
| `f` | OLCF Frontier | OpenACC (Cray ftn) | `source ./mfc.sh load -c f -m g` |
78+
| `f` | OLCF Frontier | OpenACC/OpenMP (Cray ftn) | `source ./mfc.sh load -c f -m g` |
7979
| `tuo` | LLNL Tuolumne | OpenMP (Cray ftn) | `source ./mfc.sh load -c tuo -m g` |
8080
| `d` | NCSA Delta | OpenACC (nvfortran) | `source ./mfc.sh load -c d -m g` |
8181
| `b` | PSC Bridges2 | OpenACC (nvfortran) | `source ./mfc.sh load -c b -m g` |
@@ -132,7 +132,8 @@ NEVER use raw OpenACC/OpenMP pragmas (`!$acc`, `!$omp`). Use `GPU_*` Fypp macros
132132
Raw `#ifdef`/`#ifndef` preprocessor guards for feature/compiler/library gating ARE normal.
133133
NEVER use double-precision intrinsics: `dsqrt`, `dexp`, `dlog`, `dble`, `dabs`, `real(8)`, `real(4)`.
134134
Use generic intrinsics (`sqrt`, `exp`, `log`) and precision types (`wp`, `stp`).
135-
NEVER use `stop` or `error stop`. Use `call s_mpi_abort()`.
135+
NEVER use `d` exponent literals (`1.0d0`). Use `1.0_wp` instead.
136+
NEVER use `stop` or `error stop`. Use `call s_mpi_abort()` or `@:PROHIBIT()`/`@:ASSERT()`.
136137
NEVER use `goto`, `COMMON` blocks, or global `save` variables.
137138

138139
Every `@:ALLOCATE(...)` MUST have a matching `@:DEALLOCATE(...)`.
@@ -153,7 +154,7 @@ Changes to `src/common/` affect ALL three executables. Test comprehensively.
153154

154155
## Precision System
155156

156-
- `wp` = working precision (computation). `stp` = storage precision (I/O).
157+
- `wp` = working precision (computation). `stp` = storage precision (field data arrays and I/O).
157158
- Default: both double. Single mode: both single. Mixed: wp=double, stp=half.
158159
- MPI types must match: `mpi_p``wp`, `mpi_io_p``stp`.
159160

0 commit comments

Comments
 (0)