open-mpi / ompi

Open MPI main development repository
https://www.open-mpi.org
Other
2.08k stars 845 forks source link

Add missing MPI_F08_STATUS[ES]_IGNORE #1475

Closed jsquyres closed 2 years ago

jsquyres commented 8 years ago

According to MPI-3.1 p681, we're supposed to have MPI_F08_STATUS[ES]_IGNORE in mpi.h.

ggouaillardet commented 8 years ago

@jsquyres i started this

ggouaillardet commented 8 years ago

fwiw, this is the tip of the iceberg ... MPI_Status_f082c and MPI_Status_f082f and friends is where the heavy work is

btw, why OMPI_IS_FORTRAN_STATUS_IGNORE does not simply use MPI_F_STATUS_IGNORE ?

jsquyres commented 8 years ago

@ggouaillardet Probably just so that all the declarations in mpif-c-constants-decl.h are of the same form...?

ggouaillardet commented 8 years ago

@jsquyres i just found i screwed up some time ago ...

in ompi/mpi/fortran/base/constants.h from the v1.10 branch

 * Since we made the fundamental decision to support all 4 common
 * fortran compiler symbol conventions within the same library for
 * those compilers who support weak symbols, we need to have 4 symbols
 * for each of the fortran address constants.  As described above, we
 * have to have known *pointer* values for the fortran addresses
 * (e.g., MPI_STATUS_IGNORE).  So when the fortran wrapper for
 * MPI_RECV gets (MPI_Fint *status), it can check (status ==
 * some_sentinel_value) to know that it got the Fortran equivalent of
 * MPI_STATUS_IGNORE and therefore pass the C MPI_STATUS_IGNORE to the
 * C MPI_Recv.

but later, i made some changes that you documented

 * Generation 3 / starting with v2.x: OMPI_IS_FORTRAN_foo() will check
 * just the one symbol that is relevant for your compiler/platform
 * (based on what configure figured out).

bottom line, if i build ompi master with gfortran (single underscore by default) and build my app with -fsecond-underscore, then the application builds successfully, but MPI_STATUS_IGNORE and friends are not interpreted correctly

that being said, MPI_F_STATUS_IGNORE and MPI_F08_STATUS_IGNORE cannot work as expected if we make the fundamental decision to support all 4 common fortran compiler symbol conventions within the same library

at this stage, my opinion is we cannot support all 4 common fortran compiler symbol conventions within the same library, so i'd rather fix the comment and remove all the weak symbols we do not want any more starting from gen3, and use MPI_F_STATUS_IGNORE inside OMPI_FORTRAN_IS_STATUS_IGNORE

any thoughts ?

ggouaillardet commented 8 years ago

@jsquyres from mpi 3.1 standard page 658

int MPI_Status_f2f08(MPI_Fint *f_status, MPI_F08_status *f08_status)
MPI_Status_f2f08(f_status, f08_status, ierror)
INTEGER, INTENT(IN) :: f_status(MPI_STATUS_SIZE)
TYPE(MPI_Status), INTENT(OUT) :: f08_status
INTEGER, OPTIONAL, INTENT(OUT) :: ierror
MPI_STATUS_F2F08(F_STATUS, F08_STATUS, IERROR)
INTEGER :: F_STATUS(MPI_STATUS_SIZE)
TYPE(MPI_Status) :: F08_STATUS
INTEGER IERROR

i am not sure how to interpret that ... there are C and Fortran2008 subroutines MPI_Status_f2f08 what about MPI_STATUS_F2F08 ? is this an other name of the same function from the use mpi_f08 binding ? is this a use mpi binding ? in this case, TYPE(MPI_Status) is Fortran 2008 syntax, so i am a bit puzzled ... or is this simply an error ?

omor1 commented 7 years ago

Has there been any progress on this? Note that MPI_F08_STATUS[ES]_IGNORE actually appeared in MPI-3.0 (see page 674). Technically, Open MPI isn't fully compliant with MPI-3.0 or MPI-3.1, though in practice, I doubt these constants are in much use (else I would expect there to be more of a fuss about the omission) and are most likely not a part of any test suite.

Looking at other implementations, it appears that MPICH defines these constants, though they just point them to MPI_STATUS[ES]_IGNORE and state they aren't used. Note that they accidentally mis-named the struct in previous versions (MPI_F08_Status instead of MPI_F08_status). This is true for MPICH-derived libraries as well, such as Intel MPI.

I'm currently working on an MPI binding for Swift—for now, at least, I'll leave these constants out, as it appears to be problematic in a number of implementations.

SpinTensor commented 4 years ago

Hi, are there any news regarding the inclusing of MPI_F08_Status C-type and the MPI_Status_f082c C-routine? I need them in a lot of places of a project I am working on and a workaround via F90-status transfers would be possible, but ugly and cumbersome.

SpinTensor commented 4 years ago

Hi again. While trying to implement a workaround I realized that it is also not possible to convert between f08 and f77 status variables with openmpi. This means that there is no way to in open-mpi to pass an f08 status in any way to a c-routine, either directly or inderectly via f77.

ggouaillardet commented 4 years ago

@SpinTensor I am working on it

jsquyres commented 4 years ago

Ugh; are we missing a bunch of the status conversion functions from MPI-3.1 Figure 17.1 (aka figure 33 in https://www.mpi-forum.org/docs/mpi-3.1/mpi31-report/node447.htm#Node447)? That's... embarrassing. ☹️ @ggouaillardet If you have a PR up soon, I'll be happy to test / review.

Would be Very Good to have this in time for v5.0.0 (i.e., ASAP).

ggouaillardet commented 4 years ago

@jsquyres PR was posted 3 hours ago, and I chose you as the reviewer :-)

note MPI_F08_STATUS[ES]_IGNORE is still not defined (mangling issue iirc) but all the conversion subroutines are now implemented.

jsquyres commented 3 years ago

@ggouaillardet Did you ever get a chance to look at adding the missing MPI_F08_STATUS[ES]_IGNORE symbols, perchance?

SpinTensor commented 2 years ago

Hi just wanted to check on the status. I see that ggouaillardet already started a merge request. When will the changes be available in the main trunk?

jsquyres commented 2 years ago

@SpinTensor Thanks for the reminder; this one fell off the radar. ☹️

jsquyres commented 2 years ago

This was (finally) merged to master, and the fix-the-missing-MPI_F08_STATUS[ES]_IGNORE PR is pending being merged into the v5.0.x branch.

Per https://github.com/open-mpi/ompi/issues/8047#issuecomment-693397908, we didn't bring the missing conversion routines to v4.1.x or v4.0.x because of deeper ABI implications. As an extension of that, I didn't back-port the missing MPI_F08_STATUS[ES]_IGNORE constants fix to the v4.1.x or v4.0.x branches, either. Specifically: I didn't check if there were ABI implications for the MPI_F08_STATUS[ES]_IGNORE fixes, but if we didn't bring over the somewhat-esoteric missing conversion routines to v4.1.x and v4.0.x, then let's not bring the missing MPI_F08_STATUE[ES]_IGNORE back to v4.1.x or v4.0.x, either.

All these fixes will be included in the upcoming v5.0.x release.

awlauria commented 2 years ago

This is fixed in master and v5.0, closing.