Skip to content

Add Fortran source code linting to MPAS dynamical core CI workflows - #435

Merged
kuanchihwang merged 9 commits into
ESCOMP:developmentfrom
kuanchihwang:staging/mpas-dycore-linting
Oct 21, 2025
Merged

Add Fortran source code linting to MPAS dynamical core CI workflows#435
kuanchihwang merged 9 commits into
ESCOMP:developmentfrom
kuanchihwang:staging/mpas-dycore-linting

Conversation

@kuanchihwang

@kuanchihwang kuanchihwang commented Oct 15, 2025

Copy link
Copy Markdown
Collaborator

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 adds Fortran source code linting to MPAS dynamical core CI workflows. Fortran source code linting is powered by Fortitude. Its documentation can be found here.

Note that, currently, the output of Fortitude is for informational purposes only. Linting rule violations are configured to not fail the CI workflows. It is up to the developers to decide whether to fix them.

With Fortran source code linting now in place, this PR also fixes the following linting errors:

  • C002 Interface 'function'/'subroutine' missing 'implicit none'
  • C051 Trailing backslash
  • C183 'stat' used without 'errmsg'
  • S104 Should be 0 space after the opening bracket
  • ... (and more)

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       src/dynamics/mpas/assets/fortitude_config.toml
  * Add Fortran source code linting to MPAS dynamical core CI workflows

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

M       .github/workflows/mpas_dynamical_core_ci.yml
  * Add Fortran source code linting to MPAS dynamical core CI workflows
M       src/dynamics/mpas/driver/dyn_mpas_subdriver.F90
M       src/dynamics/mpas/dyn_comp.F90
M       src/dynamics/mpas/dyn_comp_impl.F90
M       src/dynamics/mpas/dyn_coupling.F90
M       src/dynamics/mpas/dyn_coupling_impl.F90
M       src/dynamics/mpas/dyn_grid.F90
M       src/dynamics/mpas/dyn_grid_impl.F90
M       src/dynamics/mpas/dyn_procedures.F90
M       src/dynamics/mpas/tests/unit/test_dyn_mpas_procedures.pf
  * Fix linter errors about having `stat` without `errmsg` in `allocate` statements
  * Fix linter errors about interface procedures missing `implicit none`
  * Fix linter errors about trailing backslashes
  * Fix linter nitpicks about styles
  * Adjust variable declaration styles
  * Fix compiler warning about precision conversion

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_003.

…statements

Original linter error message was:
C183 'stat' used without 'errmsg'
Original linter error message was:
C002 Interface 'function' missing 'implicit none'
C002 Interface 'subroutine' missing 'implicit none'
Original linter error message was:
C051 Trailing backslash

The sequence of a trailing backslash followed by a newline will be interpreted as
a continued line by the C preprocessor (`cpp`). According to its documentation,
there is no way to prevent a backslash at the end of a line from being interpreted as
a backslash-newline sequence.

Unfortunately, some code comments contain LaTeX markups to express equations.
In LaTeX, `\\` is used to indicate the end of a row, but it is being misinterpreted
by the C preprocessor as a continued line.

Work around this problem by adding the optional length argument (e.g., `[0pt]`)
to `\\`.
Original linter error message was:
S104 Should be 0 space after the opening bracket
Do not group variable declarations that are not related together.
This compiler warning appears when MPAS dynamical core is compiled
in single precision mode:
"Conversion from REAL(4) to REAL(8) at (1) [-Wconversion-extra]"
@kuanchihwang
kuanchihwang marked this pull request as ready for review October 15, 2025 18:08
]
# When ignoring a linting rule, a reason should be provided.
ignore = [
'C003', # Temporarily ignored due to NVIDIA HPC SDK having abysmal support for Fortran 2018.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We could consider using more diplomatic language as well as adding more specificity here with something like

Temporarily ignored due to lack of support for Fortran 2018 'implicit none (external)' in the NVIDIA HPC SDK (as of the 25.9 release)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I revised the linting rule comments in e6f05e4.

@nusbaume nusbaume left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @kuanchihwang! At some point it would be good to discuss how we could eventually expand the fortitude linting across all of CAM-SIMA and/or atmospheric_physics (I have a few ideas), but for now I think this is a great start!

@kuanchihwang
kuanchihwang merged commit 4072b97 into ESCOMP:development Oct 21, 2025
18 checks passed
@kuanchihwang
kuanchihwang deleted the staging/mpas-dycore-linting branch October 21, 2025 16:52
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.

4 participants