-
Notifications
You must be signed in to change notification settings - Fork 16
Work around a GNU Fortran bug regarding unlimited polymorphic entities #366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Work around a GNU Fortran bug regarding unlimited polymorphic entities #366
Conversation
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.
There was a problem hiding this 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 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great find!
There was a problem hiding this 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).
There was a problem hiding this 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!
@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! |
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
M src/core_utils/string_core_utils.F90
M src/dynamics/mpas/driver/dyn_mpas_subdriver.F90
M test/unit/fortran/src/core_utils/test_string_core_utils.pf
Regression tests:
No changes to any existing tests with respect to the last baseline,
sima0_03_000
.