Add support to use mpi_f08 MPI module

[DusanJovic-NOAA]

Add support to use mpi_f08 MPI module

These changes are needed in order to switch to Fortran 2008 MPI module, which provides generic interfaces (overloaded MPI functions/subroutines) and avoids mismatches between actual and dummy argument lists in mpi calls. Starting with version 10 of GNU Fortran, argument mismatches are treated as errors, and must be silenced my adding '-fallow-argument-mismatch' compiler flag, which is undesirable.

If code is compiled without MPI support dummy definition of MPI_Comm is provided. This type is used instead of equivalent integer communicator in various MPI calls.

User interface changes?: No

Testing: Full regression test of the ufs-weather-model on Hera and Hercules.

ufs-weather-model PR: https://github.com/ufs-community/ufs-weather-model/pull/1147

[climbfuji]

FWIW: I ran all capgen and prebuild unit tests that are in the main ccpp-framework branch, and they all passed.

[DusanJovic-NOAA]

Both host model and physics scheme must use the same type of mpi communicator as subroutine argument (integer or MPI_Comm). In this case we changed ccpp physics dummy arguments and corresponding actual arguments in fv3atm.

[climbfuji]

For reference, the ccpp-physics PR (to the ufs/dev branch) is here: https://github.com/ufs-community/ccpp-physics/pull/160

I think @gold2718's question about interoperability mpi/mpi_f08 is important.

Looking at the changes in the ccpp-physics PR, I see a lot of really nice cleanup that has to do with calls to NCEPLIBS utilities - these are as far as I understand independent of the mpi/mpi_f08 changes. For the few places where use mpi is replaced with use mpi_f08, it would be good if we had an agreed-upon set of preprocessor flags that the build system sets, depending on whether one or the other is supposed to be used.

@gold2718 My pragmatic approach would be to implement that in a simplified way in ccpp_prebuild.py (as we did so often) as long as we have an agreed-upon solution that does not preclude, hinder or complicate the clean(er) implementation in capgen?

An interesting idea could be to task the CCPP framework with always exposing a MPI_comm DDT, which is either integer or the f08 mpi_comm type, depending on how it is configured. This way the metadata can always use MPI_comm. We'd have to think about using ifdefs for the use statements, or have CCPP expose the appropriate mpi module itself, e.g. as use mpi_ccpp ? Not sure ... to be discussed next week?

@dustinswales @peverwhee @mwaxmonsky @nusbaume @mkavulich @grantfirl

[scrasmussen]

Apologies if this is already known but these are resources that were helpful for me when looking at mpi_f08 issues and working on shim layers.

• OpenMPI's mpi_f08 datatypes module
• MPI 4.1 Standard: 3.5.1 Opaque Objects

[dustinswales]

@DusanJovic-NOAA @climbfuji @gold2718 Do we need to support F77 MPI interfaces? Maybe I'm overlooking something critical(?), but if so, we can just move from mpi -> mpi_f08 and make mpi_f08 a requirement going forward?

[DusanJovic-NOAA]

In fv3atm we will (if/when this set of PR is merged) switch to just using mpi_f08. It is supported on all platforms we currently support fv3atm. In terms of ccpp-framework/physics if there's a need to support both f77 and f08 interfaces, as Dom explained, some kind of build/preprocessing macro can be added to choose one or the other at build time.

[climbfuji]

@dustinswales @gold2718 @grantfirl @DusanJovic-NOAA How do we make progress here? I understand that this PR is holding up a set of PRs that would be good to merge. I also understand that the ccpp-framework changes should have been communicated/proposed a little earlier, but I am hoping we can come to an agreement on how to implement this in ccpp_prebuild so that it will transition smoothly into the capgen implementation.

Should I convert this PR back to draft and propose a solution that we can then iterate on by updating it (that's what draft PRs are for)?

[DusanJovic-NOAA]

@dustinswales @gold2718 @grantfirl @DusanJovic-NOAA How do we make progress here? I understand that this PR is holding up a set of PRs that would be good to merge. I also understand that the ccpp-framework changes should have been communicated/proposed a little earlier, but I am hoping we can come to an agreement on how to implement this in ccpp_prebuild so that it will transition smoothly into the capgen implementation.

Should I convert this PR back to draft and propose a solution that we can then iterate on by updating it (that's what draft PRs are for)?

@DomHeinzeller What is the recommended solution? Any solution that will allow us to compile ccpp using mpi_f08 module is acceptable.

[gold2718]

How do we make progress here?

@DusanJovic-NOAA, have you considered responding to each of my questions?

Have we polled all the current users of the CCPP Framework to see if they are okay with this change? NOAA is obviously in, I would like to hear from NCAR and NRL to see if they have any concerns.

[DusanJovic-NOAA]

How do we make progress here?

@DusanJovic-NOAA, have you considered responding to each of my questions?

Have we polled all the current users of the CCPP Framework to see if they are okay with this change? NOAA is obviously in, I would like to hear from NCAR and NRL to see if they have any concerns.

Opened issue https://github.com/NCAR/ccpp-framework/issues/528

[DusanJovic-NOAA]

How do we make progress here?

@DusanJovic-NOAA, have you considered responding to each of my questions?

Have we polled all the current users of the CCPP Framework to see if they are okay with this change? NOAA is obviously in, I would like to hear from NCAR and NRL to see if they have any concerns.

I have not polled all the current users of the ccpp framework, but if NCAR and NRL want to use mpicomm arguments defined as integers they should still be able to do that, as long as the corresponding arguments in physics schemes and in the host model code and metadata files are defined consistently. This PR only adds support for using MPI_comm DDT as a type of mpicomm argument if users choose to do so.

[climbfuji]

I can try to generalize this solution tonight so that users/host models can choose either the old MPI or new MPI interface (with the correct communicator) if that's ok?

[DusanJovic-NOAA]

This change does not force users to use one or the other. Users can still declare the type of mpicomm arguments either as integers or MPI_Comm, as long as they declare them consistently.

[DusanJovic-NOAA]

@climbfuji I do not know if you remember but you made the changes almost 2 years ago, which I just committed:

https://github.com/NCAR/ccpp-framework/pull/523/commits/03790c7eb7e199d6a04f7ff105bff0d28224c0d7

and ever since I was just keeping this branch up-to-date.

[DomHeinzeller]

Yes but physics schemes in ccpp-physics that get used by the UFS now have MPI_Comm as type. If that scheme gets used by CESM for example, and CESM still uses integers, that won’t work?

On Jan 30, 2024, at 8:44 AM, Dusan Jovic @.***> wrote:

This change does not force users to use one or the other. Users can still declare the type of mpicomm arguments either integers or MPI_Comm, as long as they declare them consistently.

— Reply to this email directly, view it on GitHub https://github.com/NCAR/ccpp-framework/pull/523#issuecomment-1917287458, or unsubscribe https://github.com/notifications/unsubscribe-auth/AN7FF5GXS6Q6V6NQE27YBRLYREINLAVCNFSM6AAAAABCCBPG3CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJXGI4DONBVHA. You are receiving this because you were mentioned.

[DusanJovic-NOAA]

Yes but physics schemes in ccpp-physics that get used by the UFS now have MPI_Comm as type. If that scheme gets used by CESM for example, and CESM still uses integers, that won’t work? … On Jan 30, 2024, at 8:44 AM, Dusan Jovic @.***> wrote: This change does not force users to use one or the other. Users can still declare the type of mpicomm arguments either integers or MPI_Comm, as long as they declare them consistently. — Reply to this email directly, view it on GitHub <#523 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AN7FF5GXS6Q6V6NQE27YBRLYREINLAVCNFSM6AAAAABCCBPG3CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJXGI4DONBVHA. You are receiving this because you were mentioned.

That's why I said as long as code and metadata in physics and host model are consistent. But that has nothing to do with the changes in this PR. My ccpp-physics PR is for ufs-community branch of physics. Obviously if/when we decide to merge ufs branch back to NCAR's authoritative repo we should decide what do we want to use as a type of mpicomm arguments, integer or MPI_Comm or maybe both (optionally) using some cpp macro. But again this has nothing to do with ccpp-framework.

[gold2718]

Yes but physics schemes in ccpp-physics that get used by the UFS now have MPI_Comm as type. If that scheme gets used by CESM for example, and CESM still uses integers, that won’t work?

This is why we need to check with the other groups. Has anyone talked to the NCAR or NRL folks about this? For the record, CESM currently has only one scheme that takes an MPI communicator as an argument.

[climbfuji]

@DusanJovic-NOAA FYI We've confirmed with all partners that we can move to mpi_f08 in CCPP. I had already tested and approved this PR, we are now waiting for the other reviewers to approve, same for the ccpp-physics PR.

[grantfirl]

@climbfuji @gold2718 If we're making MPI a hard requirement, do we need to remove all IFDEFs checking for it, in this PR and eleswhere?

[climbfuji]

Yes, I think we should do that.

[climbfuji]

Yes, I think we should do that.

EDIT: Once that decision was made (hopefully soon - today?)

@DusanJovic-NOAA Please hold off making these changes until we've made that decision.

[climbfuji]

Ok. We'll move ahead and make mpi_f08 a requirement for CCPP Framework and Physics. @DusanJovic-NOAA I'll send you a PR that updates your branch accordingly. I'll also check your ccpp-physics branch for any changes that need to be made.

We'll also need to make updates to the technical documentation and a few other places, but that's outside the scope of this PR.

Thanks everyone for your input to the discussion, and @DusanJovic-NOAA for your patience!

[dustinswales]

@climbfuji I will see the documentation is updated.

[mkavulich]

@DusanJovic-NOAA @climbfuji Should I go ahead and merge this PR now? Last framework meeting we had discussed some needed changes to the weather model PR, but it looks like those are now complete?

[climbfuji]

@DusanJovic-NOAA @climbfuji Should I go ahead and merge this PR now? Last framework meeting we had discussed some needed changes to the weather model PR, but it looks like those are now complete?

This needs to bubble up in the commit queue and go through the regression testing procedure first. Hopefully within the next two weeks!

[climbfuji]

@DusanJovic-NOAA Do you know what the timeline is for this PR and the associated ccpp-physics PR? Thanks!

[climbfuji]

@DusanJovic-NOAA PR https://github.com/NCAR/ccpp-framework/pull/548 was merged inadvertently. If there's still time before EPIC starts testing your PRs, would you mind updating your framework PR? This shouldn't affect anything. Sorry for the inconvenience.

[DusanJovic-NOAA]

@DusanJovic-NOAA PR #548 was merged inadvertently. If there's still time before EPIC starts testing your PRs, would you mind updating your framework PR? This shouldn't affect anything. Sorry for the inconvenience.

@climbfuji I updated this branch with the NCAR:main

[climbfuji]

Thanks very much @DusanJovic-NOAA. Following your PR we need to add another PR that repairs CI (doesn't have MPI dependency).

[zach1221]

Hi, @grantfirl . Testing is complete on ufs-wm PR #1147 . Please feel free to merge this PR.