Skip to content
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

Fix type mismatch compiler error when gfortran 10 is used without '-fallow-argument-mismatch' flag #1147

Merged
merged 113 commits into from
Mar 20, 2024

Conversation

DusanJovic-NOAA
Copy link
Collaborator

@DusanJovic-NOAA DusanJovic-NOAA commented Mar 29, 2022

Commit Queue Requirements:

  • Fill out all sections of this template.
  • All sub component pull requests have been reviewed by their code managers.
  • Run the full Intel+GNU RT suite (compared to current baselines) on either Hera/Derecho/Hercules
  • Commit 'test_changes.list' from previous step

Description:

Starting with version 10 gfortran treats mismatches between actual and dummy argument lists as errors. This error can be turned into warning (and silenced) by using '-fallow-argument-mismatch' flag. In this PR all such mismatches are fixed. Most of the errors are resolved by using mpi_f08 module which provides generic interfaces.

WARNING: We are currently using mpich MPI library with the gnu compilers on Hera and SGI MPT on Cheyenne. mpi_f08 module in mpich, when compiled with the current versions of the gnu compilers, has some issues and MPT does not provide mpi_f08 module at all. Which means this PR will require us to switch to OpenMPI, which will require hpc-stack to be rebuild on these two platforms. Do we want to do that? Do we want to make (working) mpi_f08 module a requirement for ufs-weather-model?

Commit Message:

* UFSWM - Fix type mismatch compiler error when gfortran 10 is used without '-fallow-argument-mismatch' flag.
  * FV3 - Fix type mismatch compiler error when gfortran 10 is used without '-fallow-argument-mismatch' flag.
    * ccpp-physics - Resolve various subroutine argument mismatches.
    * ccpp-framework - Add support to use mpi_f08 MPI module .
  * stochastic_physics - Fix type mismatch compiler error when gfortran 10 is used without '-fallow-argument-mismatch' flag.

Priority:

  • Normal

Git Tracking

UFSWM:

Sub component Pull Requests:

UFSWM Blocking Dependencies:

  • None

Changes

Regression Test Changes (Please commit test_changes.list):

  • No Baseline Changes.

Input data Changes:

  • None.

Library Changes/Upgrades:

  • No Updates

Testing Log:

  • RDHPCS
    • Hera
    • Orion
    • Hercules
    • Jet
    • Gaea
    • Derecho
  • WCOSS2
    • Dogwood/Cactus
    • Acorn
  • CI
  • opnReqTest (complete task if unnecessary)

@junwang-noaa
Copy link
Collaborator

If mpi_f08 can fix the issue of type mismatch, I think we can add mpi_f08 module as a requirement for ufs-weather-model to support gnu compiler.
@jkbk2004 would you please check if we can switch to OpenMP from SGI MPT on Cheyenne with gnu compiler and reinstall hpc-stack? Thanks

@jkbk2004
Copy link
Collaborator

@junwang-noaa I will give a try to install openmpi/gnu and see if its doable.

@climbfuji
Copy link
Collaborator

@DusanJovic-NOAA @junwang-noaa @kgerheiser I captured this and related information in the spack-stack repo: JCSDA/spack-stack#109

We will try to rebuild spack-stack on the various platforms using gcc, (LLVM) clang or apple-clang with open-mpi. If that all works, then it would good to merge the changes in this ufs-weather-model PR when the switch to spack-stack is made.

@junwang-noaa
Copy link
Collaborator

@climbfuji Have you tried rebuilding spacl_stack with gcc and open-mpi? Does that work?

@climbfuji
Copy link
Collaborator

@climbfuji Have you tried rebuilding spacl_stack with gcc and open-mpi? Does that work?

Your question is timely, as I was about to post an update here.

There are some issues with using openmpi on macOS that I started investigating last week. The problem has to do with flat namespaces versus two-level namespaces. mpich supports two-level namespaces through a configure option, openmpi does not. I am trying to achieve the same by setting appropriate linker flags when building openmpi and when building apps with openmpi.

Without two-level namespaces, there is a problem with mixing the libc++ from macOS (part of the native clang, and similar for LLVM clang) and libstdc++ from gcc, that results in exceptions not being caught correctly etc. I hope to have a good answer in the first week of May for whether switching to openmpi is a viable solution on macOS or not.

@climbfuji
Copy link
Collaborator

@junwang-noaa @DusanJovic-NOAA Good news. I was able to compile openmpi such that it mimics the two-level namespace option of mpich on macOS. We can therefore switch to openmpi for supporting mpi_f08.

@DusanJovic-NOAA
Copy link
Collaborator Author

Still waiting on gnu/openmpi hpc-stack or spack-stack to be installed on Hera and Cheyenne.

@climbfuji
Copy link
Collaborator

Still waiting on gnu/openmpi hpc-stack or spack-stack to be installed on Hera and Cheyenne.

I have a gnu-openmpi spack-stack that can be used for testing on Cheyenne, but it's probably not the final location and the responsibility for Cheyenne should probably be with EPIC, not JCSDA (but it's fine to do this in the transition period).

@zach1221 zach1221 removed hercules-RT Run Hera regression testing derecho-RT Run regression tests on Derecho labels Mar 19, 2024
@DusanJovic-NOAA
Copy link
Collaborator Author

I just updated ccpp/framework and FV3. I also submitted test on Hercules just to be sure the ccpp/framework update does not unexpectedly change something. So far several test finished successfully so I do not expect any issues.

@epic-cicd-jenkins epic-cicd-jenkins removed No Baseline Change No Baseline Change Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked. labels Mar 19, 2024
@zach1221 zach1221 added No Baseline Change No Baseline Change Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked. labels Mar 19, 2024
@zach1221
Copy link
Collaborator

Disregard the failed hercules message.

@BrianCurtis-NOAA
Copy link
Collaborator

Skip Acorn

@zach1221 zach1221 added gaea-RT and removed gaea-RT labels Mar 19, 2024
@zach1221
Copy link
Collaborator

Ok, since hera is down, we're finished with testing. I will follow up on the ccpp framework and physics sub-prs to begin the merge process.

Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

I can only really review the changes related to fv3atm / ccpp and those look good! This is probably the PR holding the record for being longest in the queue :-)

@zach1221 zach1221 merged commit 3c8338c into ufs-community:develop Mar 20, 2024
@DusanJovic-NOAA DusanJovic-NOAA deleted the no_arg_mismatch branch March 28, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Baseline Change No Baseline Change Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Fix type mismatch compiler error when gfortran 10 is used without '-fallow-argument-mismatch' flag