diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..c02a6dcb8 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,91 @@ +# atmospheric_physics AI agent guidance + +This repository (`ESCOMP/atmospheric_physics`) is the unified science codebase of CCPP-compliant atmospheric physics parameterizations, shared between **CAM** (submoduled at `src/atmos_phys`) and **CAM-SIMA** (submoduled at `src/physics/ncar_ccpp`). The default branch is `main` (the `development` branch is deprecated). + +Schemes here must remain **portable**: usable by any CCPP-enabled host model, free of host-model code. Host-side concerns (registry, build system, snapshot testing) are documented in CAM-SIMA's `AGENTS.md`; porting considerations from CAM are in CAM-SIMA's `PORTING.md`. Full conversion and usage documentation: https://escomp.github.io/CAM-SIMA-docs/. + +## If you remember nothing else + +1. **No host-model code in schemes.** Never `use` a CAM or CAM-SIMA module (see [Portability](#portability) for the allowed list and the `sima_diagnostics` exception). +2. **State answer impact in every PR.** The default expectation for ports, refactors, and cleanups is bit-for-bit: do not reorder floating-point arithmetic, change parenthesization, or "simplify" expressions as a side effect of other work. +3. **Never invent standard names silently.** Reuse blessed names (see [Standard names](#standard-names)); explicitly flag any new name in the PR description. +4. **Errors return, never abort.** Set `errmsg`/`errflg` and `return`; there is no `endrun` here. +5. **Keep `.meta` and Fortran in sync.** Every argument change must be mirrored in the companion `.meta` file. + +## AI disclosure + +atmospheric_physics **requires** disclosure of AI-assisted contributions: + +- In PR descriptions: state which model(s) were used and how. +- In commits made by agents or incorporating AI-assisted code: use `Assisted-by: model:version` (e.g., `Assisted-by: claude-opus:4.8`) +- The human contributor remains responsible for all committed code at all times. + +## Portability + +- Schemes may `use` only `ccpp_kinds`, `ccpp_constituent_prop_mod`, `ccpp_const_utils`, intra-package dependencies, and legacy utilities under `to_be_ccppized/` (excluding `ppgrid`). +- Real kind is `kind_phys` from `ccpp_kinds`, never `r8`/`shr_kind_mod`. +- Anything host-side a scheme needs (physical constants, fields, the log unit) arrives as a subroutine argument threaded by standard name, not via a `use` of a host module. +- Dummy arguments are assumed-shape (`(:,:)`), never sized with host parameters like `pcols`. +- Module-level state: minimal; prefer arguments. Module-level vertically-dimensioned arrays cannot be statically sized (the vertical dimension is a runtime value in CAM-SIMA): make them allocatable and allocate in `_init`. +- Logging: never `write` to a hardcoded or host-provided unit directly; thread the `log_output_unit` standard name as an integer argument if a scheme must print. +- **Exception:** `schemes/sima_diagnostics/` is a non-portable directory centralizing CAM-SIMA-specific history output. Code outside `sima_diagnostics/` that calls `cam_history` routines is a bug. +- `to_be_ccppized/` holds shared legacy utilities pending full CCPPization; do not add new code there without discussion. + +## Key CCPP scheme conventions + +- Subroutine names are `_`; generally, the module name is the scheme name, although one file may contain multiple schemes (e.g., `physics_tendency_updaters.F90`). +- Valid phases: `register`, `init`, `timestep_init`, `run`, `timestep_final`, `final`. All phases are optional. +- Every phase subroutine has `errmsg` and `errflg` (or `errcode`) as `intent(out)`: standard names `ccpp_error_message` / `ccpp_error_code`. These are required in every scheme. +- Two required Doxygen lines precede each subroutine. Current form uses the `arg_table_` prefix on the html: + ``` + !> \section arg_table__ Argument Table + !! \htmlinclude arg_table__.html + ``` + Older schemes omit the `arg_table_` prefix on the html file but you should use the new convention. +- Every CCPP scheme subroutine argument must have a corresponding entry in the companion `.meta` file with a valid standard name. A mismatch between the Fortran arguments and the `.meta` entries is the most common source of capgen errors. +- Constituent registration (declaring advected or non-advected constituents) happens in the `_register` phase, not `_init`. The `_register` phase runs before `_init` and before constituent indices are available. Index lookup at runtime uses `ccpp_const_get_idx` with the constituent properties object passed as an argument. + +## Standard names + +The standard name is the interface: the CCPP framework connects producers and consumers by exact standard-name match. + +- **Reuse before coining.** Search recently converted schemes in this repo and CAM-SIMA's `src/data/registry.xml` for current, blessed usage. The official dictionary ([ESCOMP/CCPPStandardNames](https://github.com/ESCOMP/CCPPStandardNames)) is authoritative but lags current usage. +- If a new name is unavoidable, follow the naming patterns of existing names and **flag it explicitly in the PR description** for SE review. Units must be consistent with existing usage. +- The horizontal dimension standard name depends on phase: `horizontal_dimension` in non-`run` phases (`init`, `timestep_init`, ...), `horizontal_loop_extent` in the `run` phase. +- Vertical dimensions: `vertical_layer_dimension` (layers) and `vertical_interface_dimension` (interfaces). + +## Code style + +Read `doc/Code-style.md` before writing or modifying Fortran. + +## Code reviews + +Read `doc/Code-review.md` for specific guidelines on code reviews when performing a code review task on pull requests within this repository. + +## Testing + +Build and run the pFUnit unit tests (also run by CI with coverage on every PR): + +``` +cmake -DCMAKE_PREFIX_PATH=/build/installed -DATMOSPHERIC_PHYSICS_ENABLE_TESTS=ON -S ./test/unit-test -B ./build +cd build && make && ctest -V +``` + +- pFUnit tests live at `test/unit-test/tests//test_.pf`, mirroring the scheme location. +- Test (i.e., single physics scheme) SDFs are located in `test/test_suites`; production (i.e., CAM4, CAM5, CAM7) SDFs are located in `suites`. +- Full-model validation (snapshot testing against CAM) runs from CAM-SIMA and requires NCAR cluster access; see CAM-SIMA's `AGENTS.md`. It cannot be run from this repo alone. + +## PRs and commits + +- PRs target `main`. Branch from a personal fork. Never push branches under `ESCOMP`. +- Feature branches are squash-merged. Do not add version tags to commit subjects; tags are applied at release time. +- Follow `.github/pull_request_template.md`, which requires listing changed files, test results, and an explicit statement of whether the PR changes answers. +- `doc/ChangeLog` and `doc/ChangeLog_template` are deprecated (history lives in git); do not add entries. + +## Example code to scope for conventions + +Learn the current conventions from recently converted schemes, which are preferred over older schemes that predate convention updates: + +* Minimal CCPP scheme: `schemes/cloud_fraction/convective_cloud_cover.{F90,meta}` +* Scheme with namelist XML: `schemes/vertical_diffusion/vertical_diffusion_sponge_layer.{F90,meta}`, `schemes/vertical_diffusion/vertical_diffusion_sponge_layer_namelist.xml` +* Diagnostics scheme: `schemes/sima_diagnostics/scheme_diagnostics_template.F90` diff --git a/doc/Code-review.md b/doc/Code-review.md new file mode 100644 index 000000000..1531a2876 --- /dev/null +++ b/doc/Code-review.md @@ -0,0 +1,39 @@ +# Code review rules + +In addition to existing rules outlined in AGENTS.md and Code-style.md, the following includes rules that should be verified when you are an AI agent performing a code review task on pull requests within this repository: + +1. No `save` statement at the module level, since module variables are implicitly `save`d. + Subroutine level implicit `save` is usually unintentional and should be flagged. + +2. Use-statements should be at the lowest necessary scope + +3. Subroutines and functions should be labeled `pure` when they satisfy the “pure” requirements defined in the Fortran standard. + +4. Use `end do` over `enddo`, and `end if` over `endif`. + +5. Use Modern Fortran syntax when applicable (e.g. `>` instead of `.gt.`) + +6. Make sure namelist XML files have the standard comment block at the beginning. + +7. Make sure all `allocate` calls return `stat` and `errmsg`, and if there is a non-zero `stat` in the physics scheme it returns with correctly set `ccpp_error_code` and `ccpp_error_message` variables. + +8. Make sure all citations have a DOI, or if not, as close to a full citation as possible. + +9. Check that all full-sentence comments are grammatically correct. + +10. Convert any subroutines that have only a single output variable to functions. + +11. Make sure all character-type subroutine arguments are `len=*`, unless they are declared as an allocatable or pointer in the subroutine itself. + +12. Make sure all `goto` and `go to` statements have been removed. + +13. Request the removal of any lines of code that are commented out + unless that code is explicity part of a larger comment block + describing currently working code. If the comments aren't clear + then simply flag it for human review. + +14. Check all variables in modified `*.meta` files and flag any variables + where the `standard_name` attribute is the same, but the `units` attribute + is different. While it may not be incorrect, it could indicate the + possibility of a science bug, and thus should be flagged so that + a human can look it over. diff --git a/doc/Code-style.md b/doc/Code-style.md new file mode 100644 index 000000000..7b2db5427 --- /dev/null +++ b/doc/Code-style.md @@ -0,0 +1,46 @@ +# Fortran code style + +Distilled, agent-relevant rules from the [CAM coding standards](https://escomp.github.io/CAM-SIMA-docs/development/cam-coding-standards/), which apply to Fortran in both atmospheric_physics and CAM-SIMA. Legacy code may not comply; follow these standards for new and modified lines, but do not reformat code you are not otherwise changing. + +## If you remember nothing else + +1. Every `real` variable and literal has an explicit kind: `1.5_kind_phys`, never `1.5` or `1.5d0`. Literals include the decimal point. +2. Never initialize a local variable on its declaration line — it acquires implicit `SAVE` and is not thread-safe. Initialize at the top of the executable section. (Module-level variables already have implicit `SAVE` per the Fortran standard; an explicit `save` there is redundant and being removed in refactors.) +3. No naked `use`: always `use mod, only: symbol`. +4. Modules have `implicit none` in the preamble and are default `private`; public interfaces are declared explicitly. +5. Optional arguments are passed by keyword: `call sub(x, opt_y=y)`, never positionally. +6. Bring `use` statements in at the smallest scope: inside subroutines, unless the symbol is needed by module-level declarations. + +## MUST + +- Spaces, not tabs; no trailing whitespace. +- `intent` on all dummy arguments except pointers. +- Fortran 90+ declaration syntax: `::` in all variable, type, and procedure declarations; `character(len=*)`-style character declarations. +- `if` statements spanning more than one line use `if ... then`; no continued single-line `if`. +- No semicolons combining statements on one line. +- Functions must not have side effects and should carry the `pure` keyword; if a function cannot be `pure`, say why in its preamble. +- Initialize local pointers (by default with `nullify`) before any non-initialization statement. +- Namelist variables (except logicals) are initialized to invalid sentinels: integer `-HUGE(1)`, real `NaN`, character `'UNSET'`. + +## SHOULD + +- Avoid preprocessor directives (`#if`, `#ifdef`); prefer runtime logic. +- Break long formulas into readable pieces with temporary variables. +- Use symbolic comparison operators (`==`, `/=`, `<`) rather than `.eq.`-style. +- Avoid pointers as dummy arguments. +- Module name matches the filename (minus `.F90`). +- One dummy argument declaration per line (related items may be grouped); declaration order matches the argument list. +- Avoid unnecessary statements (e.g., a bare `return` at the end of a subroutine). + +## Layout + +- Indentation follows scope (3 spaces recommended; each module at least self-consistent). +- Lines under 133 characters. +- Continuation lines indented 5 spaces or aligned with similar lines in the statement. +- Spaces around operators and `::`, after commas and after `if`/`else`/`end`/`do`/`while`; `only:` with no space before the colon. + +## Comments + +- Explain the purpose or non-obvious rationale of the following code; do not restate the code logic. +- When modifying code, check that adjacent comments remain correct. +- Do not keep commented-out code "for later".