Skip to content

Conversation

kuanchihwang
Copy link
Collaborator

@kuanchihwang kuanchihwang commented Sep 18, 2025

Tag name (required for release branches):

None

Originator(s):

kuanchihwang

Descriptions (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number):

This PR introduces continuous integration (CI) workflows and implements unit tests for MPAS dynamical core. Additionally, pFUnit files (*.pf) are now correctly rendered as Fortran source code on GitHub.

Beyond verifying correctness and robustness, the unit tests also report the numerical accuracy of each computational procedure in terms of units in the last place (ULP) away from the exact answers. For example, below is the output for equation of state:

2:  Start: <test_dyn_procedures_suite.test_equation_of_state_family_by_typical_values>
2: .
2: Error statistics (N = 31512303):
2:  63.81% of sample values (20107401) have accuracy of 0 <= ULP < 1.
2:  34.31% of sample values (10810918) have accuracy of 1 <= ULP < 2.
2:   1.88% of sample values (593637) have accuracy of 2 <= ULP < 3.
2:   0.00% of sample values (347) have accuracy of 3 <= ULP < 4.
2:    end: <test_dyn_procedures_suite.test_equation_of_state_family_by_typical_values>

Whenever changes are made in the path of MPAS dynamical core (src/dynamics/mpas/*), the CI workflows automatically build and run the unit tests with various versions of GCC. Test results are collected as artifacts and retained for 7 days for reference.

Describe any changes made to the build system:

None

Describe any changes made to the namelist:

None

List any changes to the defaults for the input datasets (e.g., boundary datasets):

None

List all files eliminated and why:

None

List all files added and what they do:

A       .gitattributes
  * Make GitHub Linguist recognize `*.pf` files as Fortran source code
A       .github/workflows/mpas_dynamical_core_ci.yml
  * Add CI workflow for MPAS dynamical core
A       src/dynamics/mpas/CMakeLists.txt
A       src/dynamics/mpas/tests/unit/CMakeLists.txt
A       src/dynamics/mpas/tests/unit/test_dyn_mpas_procedures.pf
A       src/dynamics/mpas/tests/unit/test_dyn_procedures.pf
  * Implement unit tests for MPAS dynamical core

List all existing files that have been modified, and describe the changes:

M       .github/workflows/fortran_unit_tests.yml
  * Exclude `main` branch from CAM-SIMA CI
  * Pin Ubuntu runner image version for better CI stability
  * Give CI job more descriptive name
  * Add pFUnit caching to CAM-SIMA CI
  * Refactor PIO build steps
  * Use `actions/checkout@v5` for all `git checkout` operations
  * Refactor unit testing steps
  * Simplify dependency installation
M       src/dynamics/mpas/driver/dyn_mpas_procedures.F90
  * Add support for comparing NaN and Inf with `almost_equal`
M       src/dynamics/mpas/dyn_comp_impl.F90
  * Use `almost_equal` for checking topography data
  * Use `qv_of_sh` for converting specific humidity to water vapor mixing ratio
M       src/dynamics/mpas/dyn_coupling_impl.F90
  * Use `exner_function` for improved readability in dynamics-physics coupling
  * Sort `use` statements
M       src/dynamics/mpas/dyn_procedures.F90
  * New interface for `exner_function`
  * Explicitly specify integers to be `int32` in `sec_to_hour_min_sec`

Regression tests:

FAIL SMS_Ln9.ne3pg3_ne3pg3_mg37.FKESSLER.derecho_intel.cam-outfrq_se_cslam_multitape NLCOMP

Except for the known failing test above, all the other tests pass with respect to the last baseline, sima0_08_002.

@kuanchihwang kuanchihwang marked this pull request as ready for review September 18, 2025 22:56
@kuanchihwang
Copy link
Collaborator Author

To reviewers, if you are interested in the CI workflow logs, look for "MPAS Dynamical Core CI" in the "Checks" tab of this PR.

Both Intel Fortran classic (ifort) and LLVM (ifx) compilers fail to compile
some, but not all, perfectly valid internal (i.e., nested) subroutines.
Versions 2024.2, 2025.0, and 2025.1 are affected.

This issue seems to be related to host associations being mishandled. Child
subroutines somehow lose access to the variables from the parent scope.
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing in all of these unit tests @kuanchihwang! I mostly just had a few comment-related requests, as well as a request that would slightly expand the scope of this PR (so feel free to say no if it is too burdensome).

Also add `staging/*` branch as a way to trigger CI in forks for testing
purposes.
`ubuntu-latest` points to the latest LTS version of Ubuntu, which is currently 24.04.
It is a moving target with a 2-year cadence. Pin runner image version and upgrade
manually as needed in the future to ensure CI stability.
In CI, we do not care about the documentation and tests of PIO.
Skip building them to save time.
Also pin RRTMGP data version for better CI stability.
When using `--output-junit` with CTest, test output will be truncated at 1024 bytes
by default, which makes the test log incomplete and not very useful. Increase the
size limit to 512 KB.
1. For Debian-based distros, before installing packages, package indexes
   should be updated first by running `apt update`.
2. `doxygen` brings many dependencies with it and takes time to install.
   In CI, we do not care about building the documentation in PIO.
   Remove it to save time.
3. `openmpi-bin` is already included in `libopenmpi-dev`. No need to
   list it twice.
@kuanchihwang kuanchihwang requested a review from nusbaume October 3, 2025 00:05
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing in my requests @kuanchihwang! This PR looks good to me now.

@kuanchihwang kuanchihwang merged commit e2f5715 into ESCOMP:development Oct 6, 2025
15 checks passed
@kuanchihwang kuanchihwang deleted the staging/mpas-dycore-unit-tests branch October 6, 2025 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants