From 290b38a3cb452f8522b20428cf9a18e29e06e34f Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Fri, 9 Oct 2026 23:22:14 -0400 Subject: [PATCH] Fix stale build, parameter, and CI claims in docs and comments - Generated-file header and NAMELIST_VARS comment said to regenerate via a cmake reconfigure; generation is a build-time custom command (cmake/ParamsCodegen.cmake). - copilot-instructions told reviewers to flag PRs that do not hand-edit the namelist/m_global_parameters, which are now generated from definitions.py; point at NAMELIST_VARS instead. Same fix in architecture.md.in. - contributing.md: drop the stale "15 files" count; '#:if MFC_SIMULATION' is a Fypp NameError in pre/post_process and false in simulation (fypp -D without a value binds None), so use '#:if defined('MFC_SIMULATION')'; cleanliness.yml only reports warning increases and never fails, so say so and drop the "Warnings increased" CI-failure row; the PR template has no GPU checklist, so point to gpuParallelization instead. - getting-started.md: 'params --family' is rejected by argparse; the flag is --families. - gpuParallelization.md: '$:$GPU_PARALLEL_LOOP' typo. - Parameter counts (~3,300 / ~3,400 / over 3,000): './mfc.sh params' reports 4211. Docs now say "over 4,000"; code docstrings and CLI help drop the number. Verified: fypp from build/venv on a scratch file reproduces the NameError and the None-is-false behavior of '#:if MFC_SIMULATION', while the defined() form selects the right branch per target; './mfc.sh params --families' works and '--family' errors; params_tests and lint_docs pass. --- .github/copilot-instructions.md | 2 +- docs/documentation/architecture.md.in | 2 +- docs/documentation/case.md | 2 +- docs/documentation/contributing.md | 14 +++++++------- docs/documentation/getting-started.md | 4 ++-- docs/documentation/gpuParallelization.md | 2 +- docs/documentation/readme.md | 2 +- toolchain/mfc/cli/commands.py | 2 +- toolchain/mfc/params/__init__.py | 4 ++-- toolchain/mfc/params/definitions.py | 4 ++-- toolchain/mfc/params/generators/fortran_gen.py | 2 +- toolchain/mfc/params_cmd.py | 2 +- 12 files changed, 21 insertions(+), 21 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index bad6e9ad45..2afec27abd 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -42,7 +42,7 @@ See the **Common Pitfalls** section of `docs/documentation/contributing.md` for Flag these patterns when reviewing a pull request: -* PR adds a parameter in `toolchain/mfc/params/definitions.py` but does not update the `namelist /user_inputs/` in `src/*/m_start_up.fpp` or declare it in `src/*/m_global_parameters.fpp` +* PR adds a parameter in `toolchain/mfc/params/definitions.py` but does not register its namelist root in `NAMELIST_VARS` (namelist bindings and scalar declarations are generated from `definitions.py` at build time, so they should not be hand-edited into `src/*/m_start_up.fpp` or `src/*/m_global_parameters.fpp`) * PR adds a parameter with cross-parameter constraints but does not add validation in `toolchain/mfc/case_validator.py` * PR modifies files in `src/common/` but does not mention testing all three targets (pre_process, simulation, post_process) * PR adds `@:ALLOCATE` calls without matching `@:DEALLOCATE` in the corresponding finalization subroutine diff --git a/docs/documentation/architecture.md.in b/docs/documentation/architecture.md.in index 48e78cf291..b4126fc6ac 100644 --- a/docs/documentation/architecture.md.in +++ b/docs/documentation/architecture.md.in @@ -111,7 +111,7 @@ To add a new source term or physics model: 1. **Create a module** in `src/simulation/` (e.g., `m_my_model.fpp`) 2. **Add initialization/finalization** subroutines called from `m_start_up` 3. **Add RHS contributions** called from the dimensional loop in `m_rhs:s_compute_rhs` -4. **Add parameters** to `m_global_parameters` and input validation to `m_checker` +4. **Add parameters** to `toolchain/mfc/params/definitions.py` (Fortran declarations and namelist bindings are generated from it) and input validation to `m_checker` 5. **Add a module-level brief** (enforced by the linter in `lint_docs.py`) 6. **Add the module to `docs/module_categories.json`** so it appears in this page diff --git a/docs/documentation/case.md b/docs/documentation/case.md index 4efcc06e03..847a758c81 100644 --- a/docs/documentation/case.md +++ b/docs/documentation/case.md @@ -78,7 +78,7 @@ Before diving into parameter details, check the **@ref case_constraints "Feature - Requirements for each model equation and Riemann solver **Parameter Lookup:** -- CLI search: `./mfc.sh params ` - Search ~3,300 parameters from the command line +- CLI search: `./mfc.sh params ` - Search over 4,000 parameters from the command line - Full reference: **@ref parameters "Case Parameters"** - Complete parameter documentation There are multiple sets of parameters that must be specified in the python input file: diff --git a/docs/documentation/contributing.md b/docs/documentation/contributing.md index 9767705a0e..9309341f7b 100644 --- a/docs/documentation/contributing.md +++ b/docs/documentation/contributing.md @@ -76,7 +76,7 @@ Key data structures (defined in `src/common/m_derived_types.fpp`): 2. **Fypp** preprocesses `.fpp` files into `.f90` (expands GPU macros, code generation) 3. **Fortran compiler** builds three executables from the generated `.f90` files -See @ref parameters for the full list of ~3,400 simulation parameters. See @ref case_constraints for feature compatibility and example configurations. +See @ref parameters for the full list of over 4,000 simulation parameters. See @ref case_constraints for feature compatibility and example configurations. ### How the Build Fits Together @@ -110,9 +110,10 @@ file sees exactly the include path for the target being compiled. `src/common/` compiled once per executable with the `MFC_` preprocessor define (`MFC_PRE_PROCESS`, `MFC_SIMULATION`, or `MFC_POST_PROCESS`) — this is intentional. It is what lets common modules include per-target generated files and gate -simulation-only code with `#ifdef MFC_SIMULATION` without duplication. +simulation-only code with `#ifdef MFC_SIMULATION` (or ``#:if defined('MFC_SIMULATION')`` in Fypp) +without duplication. -For which of the 15 files is manual vs. generated and what each contains, see the +For which files are manual vs. generated and what each contains, see the "How to Add a New Simulation Parameter" section below. ## Development Workflow @@ -762,7 +763,7 @@ Code in `src/common/` is compiled into all three executables (pre_process, simul Checklist: - Test all three targets: `./mfc.sh test` covers this -- If adding GPU code, remember that only `simulation` is GPU-accelerated. Guard GPU macros with `#:if MFC_SIMULATION` +- If adding GPU code, remember that only `simulation` is GPU-accelerated. Guard GPU macros with ``#:if defined('MFC_SIMULATION')`` - Check that new `use` statements don't create circular dependencies - New modules need `implicit none` and explicit `intent` on all arguments @@ -839,7 +840,7 @@ After the lint gate passes: - **Modes:** debug + release, MPI + no-MPI, double + single precision - **HPC runners:** Phoenix (NVIDIA/nvfortran), Frontier (AMD/Cray ftn) — both OpenACC and OpenMP backends - **Retries:** Tests retry up to 3 times before failing -- **Cleanliness check:** Compiler warnings are tracked — your PR cannot increase the warning count +- **Cleanliness check:** Compiler warnings are compared against master and any increase is reported (informational; it does not fail CI) ### Common CI Failures @@ -849,7 +850,6 @@ After the lint gate passes: | Raw pragma detected | Replace `!$acc`/`!$omp` with Fypp GPU macros (see @ref gpuParallelization) | | Double-precision intrinsic | Use generic intrinsic with `wp` kind (e.g., `sqrt` not `dsqrt`) | | Golden file mismatch | If intentional: `./mfc.sh test --generate --only ` | -| Warnings increased | Fix the new compiler warnings before merging | See @ref troubleshooting for detailed debugging workflows. @@ -868,7 +868,7 @@ See @ref troubleshooting for detailed debugging workflows. 5. **Ensure CI passes** before requesting review. Run `./mfc.sh test` locally first. Formatting and linting are handled automatically by the pre-commit hook. 6. **Describe your testing**: what you ran, which compilers/platforms you used. -If your change touches GPU code (`src/simulation/`), see the GPU checklist in the PR template. +If your change touches GPU code (`src/simulation/`), see @ref gpuParallelization and the Fypp and GPU section above. ## Code Review and Merge diff --git a/docs/documentation/getting-started.md b/docs/documentation/getting-started.md index 77288b3a4b..f1ac75f743 100644 --- a/docs/documentation/getting-started.md +++ b/docs/documentation/getting-started.md @@ -227,13 +227,13 @@ For more options, see @ref visualization "Flow Visualization" or run `./mfc.sh v ### Parameter Lookup -MFC has over 3,000 case parameters. Use the `params` command to search and explore them: +MFC has over 4,000 case parameters. Use the `params` command to search and explore them: ```shell ./mfc.sh params dt # Search for parameters matching "dt" ./mfc.sh params -d dt # Show parameter with description ./mfc.sh params patch # Find all patch-related parameters -./mfc.sh params --family # List all parameter families +./mfc.sh params --families # List all parameter families ``` ### Creating a New Case diff --git a/docs/documentation/gpuParallelization.md b/docs/documentation/gpuParallelization.md index a62317035d..c785d08acd 100644 --- a/docs/documentation/gpuParallelization.md +++ b/docs/documentation/gpuParallelization.md @@ -49,7 +49,7 @@ Note: Ordering is not guaranteed or stable, so use key-value pairing when using In order to parallelize a loop, simply place two macro calls on either end of the loop: ```C -$:$GPU_PARALLEL_LOOP(...) +$:GPU_PARALLEL_LOOP(...) {code} $:END_GPU_PARALLEL_LOOP() ``` diff --git a/docs/documentation/readme.md b/docs/documentation/readme.md index cbbcf5cfec..4fc8c2362f 100644 --- a/docs/documentation/readme.md +++ b/docs/documentation/readme.md @@ -12,7 +12,7 @@ Welcome to the Multi-component Flow Code (MFC) documentation. ## Reference - @ref equations "Equations" - Comprehensive equations reference -- @ref parameters "Case Parameters" - All ~3,400 parameters +- @ref parameters "Case Parameters" - All parameters (over 4,000) - @ref cli-reference "CLI Reference" - Command line options - @ref case_constraints "Case Creator Guide" - Feature compatibility - @ref physics_constraints "Physics Constraints" - Mathematical basis for validation rules diff --git a/toolchain/mfc/cli/commands.py b/toolchain/mfc/cli/commands.py index 11077e2730..f2bb186cc7 100644 --- a/toolchain/mfc/cli/commands.py +++ b/toolchain/mfc/cli/commands.py @@ -1350,7 +1350,7 @@ PARAMS_COMMAND = Command( name="params", help="Search and explore MFC case parameters.", - description="Search, list, and get information about MFC's ~3,300 case parameters.", + description="Search, list, and get information about MFC's case parameters.", positionals=[ Positional( name="query", diff --git a/toolchain/mfc/params/__init__.py b/toolchain/mfc/params/__init__.py index 39c7e6ffef..a7b3427696 100644 --- a/toolchain/mfc/params/__init__.py +++ b/toolchain/mfc/params/__init__.py @@ -1,7 +1,7 @@ """ MFC Parameter Schema Package (Minimal). -Single source of truth for MFC's ~3,300 case parameters. +Single source of truth for MFC's case parameters. Import Order ------------ @@ -10,7 +10,7 @@ 2. Schema classes (ParamDef, ParamType) for type definitions 3. definitions module is imported LAST to populate and freeze REGISTRY -The definitions import is a side-effect import that registers all ~3,300 +The definitions import is a side-effect import that registers all parameters with REGISTRY and then freezes it. This must happen at package import time so that any code importing from this package gets a fully populated, immutable registry. diff --git a/toolchain/mfc/params/definitions.py b/toolchain/mfc/params/definitions.py index cf5ff0a7b6..30760bd366 100644 --- a/toolchain/mfc/params/definitions.py +++ b/toolchain/mfc/params/definitions.py @@ -1,7 +1,7 @@ """ MFC Parameter Definitions (Compact). -Single file containing all ~3,300 parameter definitions using loops. +Single file containing all parameter definitions using loops. """ import re @@ -1216,7 +1216,7 @@ def _init_registry(): # When adding a new parameter: # 1. Add to definitions.py (type, constraints, etc.) — you are here # 2. Add the namelist root variable to NAMELIST_VARS with its target set -# 3. Re-run cmake to regenerate the .fpp files (cmake reconfigure) +# 3. Rebuild; the .fpp files are regenerated at build time (cmake/ParamsCodegen.cmake) NAMELIST_VARS: dict[str, set[str]] = {} diff --git a/toolchain/mfc/params/generators/fortran_gen.py b/toolchain/mfc/params/generators/fortran_gen.py index 605c97a9fa..adcc04d708 100644 --- a/toolchain/mfc/params/generators/fortran_gen.py +++ b/toolchain/mfc/params/generators/fortran_gen.py @@ -11,7 +11,7 @@ TARGETS = [("pre", "pre_process"), ("sim", "simulation"), ("post", "post_process")] TARGET_FROM_DIR = {full: short for short, full in TARGETS} -_HEADER = "! AUTO-GENERATED - do not edit directly. Regenerate: cmake reconfigure\n!\n" +_HEADER = "! AUTO-GENERATED - do not edit directly. Regenerated at build time (cmake/ParamsCodegen.cmake)\n!\n" _MAX_LINE = 130 _FIRST_PREFIX = "namelist /user_inputs/ " _CONT_PREFIX = " & " diff --git a/toolchain/mfc/params_cmd.py b/toolchain/mfc/params_cmd.py index e40fdfc874..6e84a42e8a 100644 --- a/toolchain/mfc/params_cmd.py +++ b/toolchain/mfc/params_cmd.py @@ -1,7 +1,7 @@ """ MFC Parameter Search and Discovery Command. -Provides CLI access to search and explore MFC's ~3,300 case parameters. +Provides CLI access to search and explore MFC's case parameters. """ import re