Allow to optionally build stochastic_physics - #281
Conversation
|
Hi @guoqing-noaa - thanks for this PR! Just confirming that the failures on the stochastic physics CI tests, specifically in running the feature code, are expected? I believe they are given your code design, but just wanted to confirm. |
There was a problem hiding this comment.
Pull request overview
This PR introduces an optional STOCHASTIC_PHYSICS build option so the atmosphere core can be compiled with or without stochastic physics support (useful for workflows that do not need the stochastic submodule, e.g., MPAS-JEDI in RRFSv2).
Changes:
- Adds
STOCHASTIC_PHYSICSas a CMake option and wires it into compile definitions / target linking. - Updates the GNU Make build path to conditionally build/link stochastic physics artifacts and include paths.
- Wraps stochastic-physics registry entries and Fortran call sites with preprocessor guards.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/core_atmosphere/Registry.xml |
Conditionally includes stochastic registry content and the stoch_pattern_sppt variable. |
src/core_atmosphere/mpas_atm_core.F |
Guards stochastic init/advance calls and module imports behind STOCHASTIC_PHYSICS. |
src/core_atmosphere/dynamics/mpas_atm_time_integration.F |
Guards stochastic pattern application to physics tendencies. |
src/core_atmosphere/Makefile |
Makes stochastic subdir build/link conditional and adjusts include paths/clean behavior. |
src/core_atmosphere/CMakeLists.txt |
Conditionally builds/link stochastic_physics library and adds compile definition. |
src/core_atmosphere/build_options.mk |
Adds -DSTOCHASTIC_PHYSICS when STOCHASTIC_PHYSICS=true for make builds. |
CMakeLists.txt |
Adds top-level option(STOCHASTIC_PHYSICS ...). |
build.sh |
Adds -s flag to enable stochastic physics for CMake builds. |
Suppressed comments (3)
src/core_atmosphere/mpas_atm_core.F:306
- As written, stochastic pattern initialization is compiled in whenever STOCHASTIC_PHYSICS is defined, even for dycore-only builds (DO_PHYSICS undefined) where the pattern is never applied. Guard this block on DO_PHYSICS too to avoid unnecessary work and dependencies.
#ifdef STOCHASTIC_PHYSICS
src/core_atmosphere/mpas_atm_core.F:1068
- Same as initialization: the stochastic_physics module import in atm_do_timestep can be avoided for dycore-only builds. Guard this
usewith DO_PHYSICS as well so STOCHASTIC_PHYSICS doesn’t introduce an unused dependency when physics is disabled.
#ifdef STOCHASTIC_PHYSICS
use mpas_stochastic_physics, only : stochastic_physics_pattern_adv, dosppt
#endif
src/core_atmosphere/mpas_atm_core.F:1105
- Advancing the stochastic pattern each timestep is only useful when DO_PHYSICS is enabled (since the pattern is applied to physics tendencies in atm_time_integration). Guard this block on DO_PHYSICS too to avoid dead work when building without physics.
#ifdef STOCHASTIC_PHYSICS
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
General comment: since this PR establishes a STOCHASTIC_PHYSICS = False baseline, should this PR also include changes for the non-stochastic CI run tests (i.e., not the compile tests) to remove the netCDF, LAPACK, BLAS, etc. routines that were added to allow for them to work with the stochastic physics enabled? |
|
@clark-evans I will update CI tests soon. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
remove the NetCDF, BLAS/MKL installation for the non-stochastic tests
|
@clark-evans As expected, quite a few tests failed. I removed the NetCDF, BLAS/MKL installation for the non-stochastic tests and hence the baseline build and test are expected to fail. |
|
All build tests now passed. Stochastic run tests all passed as well. As expected, all non-stochastic run tests failed at the step of building the baseline code. Since this PR does not modify source code, I believe we can live with it and those run tests will run correctly on future PRs. Now we only need to load compiler, mpi, pnetcdf modules and then we can compile non-stochastic MPAS-Model successfully The following extra modules will be needed to compile with stochastic_physics (note, PR #277 is needed) |
|
@clark-evans @AndersJensen-NOAA Just want to double check before merging? |
|
Good with me. A couple of the actions in the new/updated CI workflows use the deprecated Node.js 20 code, but we can clean that up (along with all of the others that I didn't get before) in a follow-up PR. |
03b2c53
into
ufs-community:noaa/develop
This PR allows to switch on/off the compiling of stochastic_physics with an build option
STOCHASTIC_PHYSICSExamples:
This also helps the MPAS-JEDI build as, in RRFSv2, JEDI does not integrate the model and hence does not need the stochastic capability.
Mandatory Questions
Reviews