"use mpi_f08" instead of "include mpif.h"

[Leonard-Reuter]

First of all thanks for the great unit test environment!

Reading Halo_mod I noticed, that "include mpif.h" is used.
(Other modules like PF_MpiTestCase already have "use mpi")

Using mpi with "include mpif.h" is outdated and should not be done anymore. Instead think about putting "use mpi" or even better (for all functionality) "use mpi_f08".

From the mpi forum:

Instead of using mpif.h, the use of the mpi_f08 or mpi module is strongly encouraged for the following reasons:

• Most mpif.h implementations do not include compile-time argument checking.
• Therefore, many bugs in MPI applications remain undetected at compile-time, such as:
  • Missing ierror as last argument in most Fortran bindings.
  • Declaration of a status as an INTEGER variable instead of an INTEGER array with size MPI_STATUS_SIZE.
  • Incorrect argument positions; e.g., interchanging the count and datatype arguments.
  • Passing incorrect MPI handles; e.g., passing a datatype instead of a communicator.
• The migration from mpif.h to the mpi module should be relatively straightforward (i.e., substituting include 'mpif.h' after an implicit statement by use mpi before that implicit statement) as long as the application syntax is correct.
• Migrating portable and correctly written applications to the mpi module is not expected to be difficult. No compile or runtime problems should occur because an mpif.h include file was always allowed to provide explicit Fortran interfaces.

[ZedThree]

Support for the mpi_f08 module is dependent on the combination of compiler and MPI implementation, but it does now seem to be available more often than not.

If moving to a hard dependency on mpi_f08 is not desired, then it could be done conditionally, and the few use mpi statements and variable types that need updating could be done via the preprocessor, for example:

#if USE_MPI_F08
# define PF_MPI_MODULE mpi_f08
# define PF_MPI_COMM_TYPE type(MPI_Comm)
#else
# define PF_MPI_MODULE mpi
# define PF_MPI_COMM_TYPE integer
#end

module PF_MpiContext
   use PF_ParallelContext
   use PF_ExceptionList, only: throw
   use PF_MPI_MODULE
   implicit none
   private
   public :: MpiContext

   type, extends(ParallelContext) :: MpiContext
      private
      PF_MPI_COMM_TYPE :: mpiCommunicator = MPI_COMM_NULL
      integer :: root = 0

It looks like there's only 5 files that would need modifying, and 3 of those only need the module name changing.

[tclune]

What about the methods that say provide the MPI communicator inside a test? The user's code (and tests) may still be using integers for the communicator and the framework needs to support that. I would think we'd want/need a (conditional) extra method that can return an MPI_Comm while preserving the interface that returns the integer.

[ZedThree]

Fair point -- I had assumed the USE_MPI_F08 condition would be selected by the user to provide compatibility when they use mpi_f08 themselves, but perhaps it might be best to always use mpi_f08 if it's available?

The standard says that the mpi_f08 derived types "are defined as Fortran BIND(C) derived types that consist of only one element INTEGER :: MPI_VAL", which means conversion to and from the integer handle turns out to be pretty trivial:

   function getMpiCommunicator(this) result(mpiCommunicator)
      integer :: mpiCommunicator
      class (MpiContext), intent(in) :: this
      mpiCommunicator = this%mpiCommunicator%MPI_VAL
   end function getMpiCommunicator

   function getMpiF08Communicator(this) result(mpiCommunicator)
      type(MPI_Comm) :: mpiCommunicator
      class (MpiContext), intent(in) :: this
      mpiCommunicator = this%mpiCommunicator
   end function getMpiF08Communicator

[tclune]

My main concern was just making sure that the default behavior remains backward compatible. If the default remains "not F08" then we can let the user update the tests themselves if they select F08. Then is some mystic future when pFUnit 5.0 comes out we would make F08 the default and not support older MPI's.

This would obviate the need for two "getters" as above. But having both might be friendlier to a gradual update.

Sound ok?