Skip to content

Commit e35a99b

Browse files
committed
docs: add Working Style section, dedupe agent guidance, fix precheck count
1 parent 303c53f commit e35a99b

3 files changed

Lines changed: 27 additions & 49 deletions

File tree

.claude/rules/common-pitfalls.md

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@
3838
- Boundary condition symmetry requirements must be maintained
3939

4040
## 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
41+
- See the compiler-backend matrix in `.claude/rules/gpu-and-mpi.md` for which compilers
42+
are CI-gated and which backends each supports.
4343
- Each compiler has different strictness levels and warning behavior
4444
- Fypp macros must expand correctly for both GPU and CPU builds
4545

@@ -54,12 +54,8 @@
5454
- Do not regenerate ALL golden files unless you understand every output change
5555

5656
## 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)
57+
The base loop (format → precheck → build → test → one logical commit) is the
58+
Development Workflow Contract in `CLAUDE.md`. Beyond it, watch for:
6259
- [ ] If adding parameters: definitions.py (_r + _nv) updated; cmake reconfigured; case_validator.py if constraints
6360
- [ ] If modifying `src/common/`: all three targets tested
64-
- [ ] If changing output: golden files regenerated for affected tests
65-
- [ ] One logical change per commit
61+
- [ ] If changing output: golden files regenerated for affected tests (`./mfc.sh test --generate --only <tests>`)

.claude/rules/fortran-conventions.md

Lines changed: 1 addition & 5 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

CLAUDE.md

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ toolchain for building/running/testing, and supports GPU acceleration via OpenAC
66
OpenMP target offload. It must compile with gfortran, nvfortran, Cray ftn, and Intel ifx (CI-gated).
77
AMD flang is additionally supported for OpenMP target offload GPU builds.
88

9+
## Working Style
10+
11+
Make surgical changes: every changed line should trace to the request. Don't refactor,
12+
reformat, or "improve" adjacent code — in a four-compiler, golden-file-gated codebase,
13+
incidental edits are how regressions slip in. For general behavioral guidance (simplicity,
14+
surfacing assumptions, verifiable success criteria), invoke the `karpathy-guidelines` skill.
15+
916
## Commands
1017

1118
Prefer using `./mfc.sh` as the entry point for building, running, testing, formatting,
@@ -39,7 +46,7 @@ All commands run from the repo root via `./mfc.sh`.
3946
./mfc.sh test --generate --only <feature> # Regenerate golden files after intentional output change
4047

4148
# Verification (pre-commit CI checks)
42-
./mfc.sh precheck -j 8 # Run all 6 lint checks (same as CI gate)
49+
./mfc.sh precheck -j 8 # Run all 7 checks (same as CI gate)
4350
./mfc.sh format -j 8 # Auto-format Fortran (.fpp/.f90) + Python
4451
./mfc.sh lint # Ruff lint + Python unit tests
4552
./mfc.sh spelling # Spell check
@@ -58,38 +65,16 @@ source ./mfc.sh load -c p -m c # Load Phoenix CPU modules
5865

5966
## System Identification and Module Loading
6067

61-
MFC targets HPC clusters. Before building on a cluster, load the correct modules
62-
via `source ./mfc.sh load -c <slug> -m <mode>`.
63-
64-
To identify the current system, check multiple signals — hostname alone is not always
65-
sufficient (compute nodes may differ from login nodes):
66-
67-
```bash
68-
hostname # e.g., login-phoenix-gnr-2.pace.gatech.edu
69-
echo $LMOD_SYSHOST # e.g., "phoenix" (most reliable when set)
70-
echo $CRAY_LD_LIBRARY_PATH # Non-empty → Cray system (Frontier, Carpenter Cray)
71-
echo $MODULESHOME # Confirms module system is available
72-
```
73-
74-
Supported systems and their slugs (full list in `toolchain/modules`):
75-
76-
| Slug | System | GPU Backend | Example |
77-
|------|--------|-------------|---------|
78-
| `p` | GT Phoenix | OpenACC (nvfortran) | `source ./mfc.sh load -c p -m g` |
79-
| `f` | OLCF Frontier | OpenACC/OpenMP (Cray ftn) | `source ./mfc.sh load -c f -m g` |
80-
| `tuo` | LLNL Tuolumne | OpenMP (Cray ftn) | `source ./mfc.sh load -c tuo -m g` |
81-
| `d` | NCSA Delta | OpenACC (nvfortran) | `source ./mfc.sh load -c d -m g` |
82-
| `b` | PSC Bridges2 | OpenACC (nvfortran) | `source ./mfc.sh load -c b -m g` |
83-
| `cc` | DoD Carpenter (Cray) | CPU only | `source ./mfc.sh load -c cc -m c` |
84-
| `c` | DoD Carpenter (GNU) | CPU only | `source ./mfc.sh load -c c -m c` |
85-
| `o` | Brown Oscar | OpenACC (nvfortran) | `source ./mfc.sh load -c o -m g` |
86-
| `h` | UF HiPerGator | OpenACC (nvfortran) | `source ./mfc.sh load -c h -m g` |
87-
88-
The `-m` flag selects mode: `g`/`gpu` for GPU builds, `c`/`cpu` for CPU-only.
89-
Batch job templates for `./mfc.sh run -e batch -c <system>` are in `toolchain/templates/`.
68+
On an HPC cluster, load modules before building: `source ./mfc.sh load -c <slug> -m <mode>`
69+
(`-m g`/`gpu` or `c`/`cpu`). The `source` is required — plain `./mfc.sh load` errors, since
70+
the command sets environment variables in the current shell.
9071

91-
IMPORTANT: `source` (or `.`) is required for `load` — it sets environment variables
92-
in the current shell. Using `./mfc.sh load` without `source` will error.
72+
Slugs live in `toolchain/modules` (e.g. `p` Phoenix, `f` Frontier, `tuo` Tuolumne, `d` Delta,
73+
`b` Bridges2, `c`/`cc` Carpenter GNU/Cray, `o` Oscar, `h` HiPerGator; GPU backend per system
74+
is defined there). To identify the current system, check `$LMOD_SYSHOST` (most reliable),
75+
then a non-empty `$CRAY_LD_LIBRARY_PATH` (→ Cray: Frontier / Carpenter-Cray), then `hostname`
76+
— login and compute nodes may differ. Batch templates for `./mfc.sh run -e batch -c <system>`
77+
are in `toolchain/templates/`.
9378

9479
## Development Workflow Contract
9580

@@ -99,7 +84,7 @@ IMPORTANT: Follow this loop for ALL code changes. Do not skip steps.
9984
2. **Plan** — For multi-file changes, outline your approach before implementing.
10085
3. **Implement** — Make small, focused changes. One logical change per commit.
10186
4. **Format** — Run `./mfc.sh format -j 8` to auto-format.
102-
5. **Verify** — Run `./mfc.sh precheck -j 8` (same 6 checks as CI lint gate).
87+
5. **Verify** — Run `./mfc.sh precheck -j 8` (same 7 checks as CI lint gate).
10388
6. **Build** — Run `./mfc.sh build -j 8` to verify compilation.
10489
7. **Test** — Run relevant tests: `./mfc.sh test --only <feature> -j 8`.
10590
For changes to `src/common/`, test ALL three targets: `./mfc.sh test -j 8`.
@@ -153,13 +138,14 @@ Changes to `src/common/` affect ALL three executables. Test comprehensively.
153138
- Modules: `m_<feature>` (e.g., `m_bubbles`)
154139
- Public subroutines: `s_<verb>_<noun>` (e.g., `s_compute_pressure`)
155140
- Public functions: `f_<verb>_<noun>`
141+
- Private/local variables: no prefix required. Constants: descriptive names, not ALL_CAPS.
156142
- 2-space indentation, lowercase keywords, explicit `intent` on all arguments
157143

158144
## Precision System
159145

160146
- `wp` = working precision (computation). `stp` = storage precision (field data arrays and I/O).
161-
- Default: both double. Single mode: both single. Mixed: wp=double, stp=half.
162-
- MPI types must match: `mpi_p``wp`, `mpi_io_p``stp`.
147+
- Both double by default. See `.claude/rules/fortran-conventions.md` for single/mixed
148+
modes, casting rules, and MPI type matching (`mpi_p``wp`, `mpi_io_p``stp`).
163149

164150
## Code Review Priorities
165151

0 commit comments

Comments
 (0)