Skip to content

Conversation

kuanchihwang
Copy link
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):

The stringify utility function uses an advanced Fortran language feature: Polymorphism. Unfortunately, recent GNU Fortran versions (>= 12) have problems with it. When a character string array is passed as the actual argument to an unlimited polymorphic dummy argument, its array index and length parameter are mishandled.

This compiler bug is perhaps the manifestation of GCC Bugzilla Bug 100819. A workaround, which is implemented by this PR, is to use a temporary character string array. As a result, the unit tests no longer need to be guarded by the __GNUC__ macro.

Additionally, this PR also harmonizes the code style in unit tests.

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:

None

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

  • M .github/workflows/fortran_unit_tests.yml
    • Avoid hard-coded paths in GitHub Actions
  • M src/core_utils/string_core_utils.F90
    • Work around a GNU Fortran bug regarding unlimited polymorphic entities
  • M src/dynamics/mpas/driver/dyn_mpas_subdriver.F90
    • Work around a GNU Fortran bug regarding unlimited polymorphic entities
  • M test/unit/fortran/src/core_utils/test_string_core_utils.pf
    • Remove condition on GNU Fortran
    • Harmonize code style in unit tests

Regression tests:

No changes to any existing tests with respect to the last baseline, sima0_03_000.

Part 1 of 2 for MPAS dynamical core.

Workaround for a bug in GNU Fortran >= 12. This is perhaps the manifestation of GCC Bugzilla Bug 100819.
When a character string array is passed as the actual argument to an unlimited polymorphic dummy argument,
its array index and length parameter are mishandled.
Part 2 of 2 for CAM-SIMA.

Workaround for a bug in GNU Fortran >= 12. This is perhaps the manifestation of GCC Bugzilla Bug 100819.
When a character string array is passed as the actual argument to an unlimited polymorphic dummy argument,
its array index and length parameter are mishandled.
Also, avoid exceeding the line length limit of 132 characters.
GitHub documentation strongly recommends that actions should use variables to
access the filesystem rather than using hard-coded paths.
Copy link
Collaborator

@mwaxmonsky mwaxmonsky left a comment

Choose a reason for hiding this comment

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

Just a quick question but otherwise all looks great!

-DCMAKE_PREFIX_PATH=/home/runner/work/CAM-SIMA/CAM-SIMA/pFUnit/build/installed \
-DCMAKE_PREFIX_PATH=$GITHUB_WORKSPACE/pFUnit/build/installed \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great find!

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 the fix @kuanchihwang! My only request is to, at least for now, revert back to the len=* syntax in the pFUnit code, as that matches much of the rest of CAM-SIMA and atmospheric_physics (although long-term I am happy to have more discussion on what the ideal Fortran style should be for SIMA as whole).

@kuanchihwang kuanchihwang requested a review from nusbaume March 4, 2025 19:26
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.

Everything looks great to me now. Thanks again @kuanchihwang!

@nusbaume
Copy link
Collaborator

nusbaume commented Mar 4, 2025

@kuanchihwang Also feel free to merge this in whenever you are ready, as it would probably be best for this PR to come in next. Thanks!

@kuanchihwang kuanchihwang merged commit 16b735a into ESCOMP:development Mar 4, 2025
12 checks passed
@kuanchihwang kuanchihwang deleted the staging/workaround-gftn-ul-poly-char branch March 4, 2025 20:42
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