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

MPI: use mpi module instead of Fortran include statement #389

Merged

Conversation

phil-blain
Copy link
Member

@phil-blain phil-blain commented Dec 9, 2019

PR checklist

  • Short (1 sentence) summary of your PR:
    Switch to mpi Fortran module instead of Fortran include statement
  • Developer(s):
    P Blain.
  • Suggest PR reviewers from list in the column to the right. @eclare108213 @apcraig
  • Please copy the PR test results link or provide a summary of testing completed below.
    I did checked that it compiles correctly, was not sure if full-blown tests were necessary.
  • How much do the PR code changes differ from the unmodified code?
    • bit for bit -> should be
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on Icepack or any other models?
    • Yes
    • No
  • Does this PR add any new test cases?
    • Yes
    • No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/. A test build of the technical docs will be performed as part of the PR testing.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please provide any additional information or relevant details below:

This is a first step towards #363. I tried to use the mpi_f08 module but this required more extensive changes to the code, so I chose to do it incrementally by first switching to the mpi module.

I had done this a while ago bu had not submitted it yet.

Copy link
Contributor

@apcraig apcraig left a comment

Choose a reason for hiding this comment

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

I have tested this on izumi and it seems to work fine. I am sort of curious about the changes in the MPI calls (MPI_Send and MPI_Abort), why they worked before, and why they work with the updated arguments. Are there multiple supported versions of the interfaces in the implementation even though that does not seem to be documented?

@apcraig apcraig merged commit 1071c7b into CICE-Consortium:master Dec 19, 2019
@phil-blain
Copy link
Member Author

The reason why it compiled correctly before is because the old way for CICE to have MPI functionality (include 'mpif.h') does not allow the compiler to check that subroutine interfaces are correctly used.

So I think all along we were not correctly using these subroutines. Since they are now defined in a module, the compiler automatically generates an interface and can thus check that they are called correctly. Citing the latest MPI specification (emphasis added):

MPI defines three methods of Fortran support:

  1. USE mpi_f08: This method is described in Section Fortran Support Through the mpi_f08 Module . It requires compile-time argument checking with unique MPI handle types and provides techniques to fully solve the optimization problems with nonblocking calls. This is the only Fortran support method that is consistent with the Fortran standard (Fortran 2008 + TS 29113 and later). This method is highly recommended for all MPI applications.
  2. USE mpi: This method is described in Section Fortran Support Through the mpi Module and requires compile-time argument checking. Handles are defined as INTEGER. This Fortran support method is inconsistent with the Fortran standard, and its use is therefore not recommended. It exists only for backwards compatibility.
  3. INCLUDE 'mpif.h': This method is described in Section Fortran Support Through the mpif.h Include File . The use of the include file mpif.h is strongly discouraged starting with MPI-3.0, because this method neither guarantees compile-time argument checking nor provides sufficient techniques to solve the optimization problems with nonblocking calls, and is therefore inconsistent with the Fortran standard. It exists only for backwards compatibility with legacy MPI applications.

The reason it ran correctly before... that's murkier. It's kind of above my technical knowledge, but I can speculate:

  • MPI_Abort : we were not sending the 3rd argument, so our ierr was interpreted as the ERRORCODE argument. I guess whatever set the IERROR argument inside the MPI implementation did not complain that it was not given this argument... maybe it was writing it at a random location in memory.
  • MPI_SEND: we were not sending the last argument, so our status flag was used inside MPI_SEND functions as IERROR. The additional argument ierr was probably ignored at compilation time.
  • MPI_GROUP_RANGE_INCL, where I had to correct the dimension of the range argument: I guess the memory layout for integer, dimension(3) and integer, dimension (3,1) is the same or close enough so it worked...

@phil-blain
Copy link
Member Author

Also, the MPI_Send calls worked fine unmodified using the Intel 2016 compiler, but Intel 2019 complained. So both the MPI implementation and the compiler vendor and version can influence how the MPI specification is enforced.

@apcraig
Copy link
Contributor

apcraig commented Dec 19, 2019

@phil-blain, thanks for the clarifications/speculation. That makes sense to me. It's surprising to me that there was never a problem before. These have probably been implemented incorrectly for a decade or more!

@phil-blain phil-blain deleted the mpi-interface-use-mpi-module branch February 12, 2020 15:08
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.

2 participants