ecmwf-ifs / fiat

The Fortran IFS and Arpege Toolkit
Apache License 2.0
7 stars 19 forks source link

MPI_F08 support for the MPL interface. #21

Open marsdeno opened 1 month ago

marsdeno commented 1 month ago
FussyDuck commented 1 month ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

wdeconinck commented 1 month ago

Hi @marsdeno I am very happy for this development. However this branch does not yet work with -DENABLE_MPI=OFF

fiat/src/fiat/mpl/internal/mpl_mpif.F90:12:5:

   12 | USE MPI_F08
      |     1
Fatal Error: Cannot open module file 'mpi_f08.mod' for reading at (1): No such file or directory
compilation terminated.

Normally it should then use the 'mpi_serial' dummy implementations. So I believe what you need is an mpi_f08 wrapper for mpi_serial as well

And with -DENABLE_MPI=OFF -DENABLE_MPL_F77_DEPRECATED=ON:

[ 29%] Building Fortran object src/fiat/CMakeFiles/fiat.dir/util/ec_meminfo.F90.o
fiat/src/fiat/util/ec_meminfo.F90:115:15:

  115 | TYPE(MPI_COMM) :: ICOMM
      |               1
Error: Derived type 'mpi_comm' at (1) is being used before it is defined
fiat/src/fiat/util/ec_meminfo.F90:116:17:

  116 | TYPE(MPI_STATUS)   :: IRECV_STATUS
      |                 1
Error: Derived type 'mpi_status' at (1) is being used before it is defined
fiat/src/fiat/util/ec_meminfo.F90:141:7:

  141 | ICOMM%MPI_VAL = KCOMM
      |       1
Error: Symbol 'icomm' at (1) has no IMPLICIT type
fiat/src/fiat/util/ec_meminfo.F90:158:27:

  158 |    CALL MPI_COMM_RANK(ICOMM,MYPROC,ERROR)
      |                           1
Error: Symbol 'icomm' at (1) has no IMPLICIT type; did you mean 'kcomm'?
fiat/src/fiat/util/ec_meminfo.F90:268:83:

  268 |             CALL MPI_RECV(LASTNODE,LEN(LASTNODE),MPI_BYTE,I,ITAG,ICOMM,IRECV_STATUS,ERROR)
      |                                                                                   1
Error: Symbol 'irecv_status' at (1) has no IMPLICIT type
marsdeno commented 1 month ago

@wdeconinck Indeed I will take care of these issues, thanks.

marsdeno commented 1 month ago

I am not an expert at all in the latest MPI standard, so I cannot discuss the pros and cons of MPI 2008 vs the traditional F77 MPI.

Regarding the changes in the interfaces, I agree they are so small that we can switch directly to MPL with MPI 2008 in ARPEGE/IFS. The code of lfidiff.F90 has been commented out. This is a problem for us, so please restore it, and do not compile it if it is a problem for you.

I would like to have the checks on non-contiguous arrays to remain as a run time option. I agree that this has a cost, and probably has to be disabled for reaching full performance, but I think that keeping the ability to check for non-contiguous arrays entering the communications is something useful.

Eventually, I think it would be extremely profitable to rewrite some of these routines (in particular mpl_send/mpl_recv) with fypp, as the length of the code makes it very difficult to check for correctness.

Will do, regarding lfidiff.F90. Noted, for the checks on contiguity. Would you think run time option is really preferable over compile time option? And finally, I do agree on the fypp question, a lot of this code is a great use case for it. For a future PR :-)