Skip to content

v2.x: mpi/fortran: use conformant dummy names for Fortran bindings #5131

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

Merged

Conversation

jsquyres
Copy link
Member

@jsquyres jsquyres commented May 2, 2018

The MPI spec defines that the "mpi" and "mpi_f08" module Fortran
bindings support passing by parameters by name. Hence, we need to use
the MPI-spec-defined parameter names ("dummy variables", in Fortran
parlance) for the "mpi" and "mpi_f08" modules.

Specifically, Fortran allows calls to procedures to be written with
keyword arguments, e.g., "call mpi_sizeof(x=x,size=rsize,ierror=ier)"
An "explicit interface" for the procedure must be in scope for this to
be allowed in a Fortran program unit. Therefore, the explicit
interface blocks we provide in the "mpi" and "mpi_f08" modules must
match the MPI published standard, including the names of the dummy
variables (i.e., parameter names), as that is how Fortran programs may
call them.

Note that we didn't find this issue previously because even though the
MPI spec allows for name-based parameter passing, not many people
actually use it. I suspect that we might have some more incorrect
parameter names -- we should probably do a full "mpi" / "mpi_f08"
module parameter name audit someday.

Thanks to Themos Tsikas for reporting the issue and supplying the
initial fix.

Signed-off-by: [email protected]
Signed-off-by: Jeff Squyres [email protected]
Signed-off-by: Gilles Gouaillardet [email protected]
(cherry picked from commit 4d126c1)

@hppritcha This has NEWS on it because we changed the Fortran interface parameter names. This is an ABI-breaking change, but honestly, no one could have been using these names before (because they were wrong).

@ThemosTsikas FYI

The MPI spec defines that the "mpi" and "mpi_f08" module Fortran
bindings support passing by parameters by name.  Hence, we need to use
the MPI-spec-defined parameter names ("dummy variables", in Fortran
parlance) for the "mpi" and "mpi_f08" modules.

Specifically, Fortran allows calls to procedures to be written with
keyword arguments, e.g., "call mpi_sizeof(x=x,size=rsize,ierror=ier)"
An "explicit interface" for the procedure must be in scope for this to
be allowed in a Fortran program unit.  Therefore, the explicit
interface blocks we provide in the "mpi" and "mpi_f08" modules must
match the MPI published standard, including the names of the dummy
variables (i.e., parameter names), as that is how Fortran programs may
call them.

Note that we didn't find this issue previously because even though the
MPI spec *allows* for name-based parameter passing, not many people
actually use it.  I suspect that we might have some more incorrect
parameter names -- we should probably do a full "mpi" / "mpi_f08"
module parameter name audit someday.

Thanks to Themos Tsikas for reporting the issue and supplying the
initial fix.

Signed-off-by: [email protected]
Signed-off-by: Jeff Squyres <[email protected]>
Signed-off-by: Gilles Gouaillardet <[email protected]>
(cherry picked from commit 4d126c1)
@ggouaillardet
Copy link
Contributor

@jsquyres In my view, the MPI forum would ideally provide mpi.h and Fortran modules (interfaces only) so we could automate conformance to the standard instead of manually checking our code vs a pdf file.

Any idea on why the MPI forum only provides a pdf file ?

@hppritcha hppritcha merged commit deb66bf into open-mpi:v2.x May 7, 2018
@jsquyres jsquyres deleted the pr/v2.x/fix-fortran-dummy-param-names branch May 7, 2018 18:45
@jsquyres
Copy link
Member Author

jsquyres commented May 7, 2018

@ggouaillardet Probably because the Forum has only ever provided a document -- not code...? Keep in mind that there's wide latitude in what needs to be in mpif.h...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants