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: Add MPI_LOGICAL{1,2,4,8,16} and MPI_TYPECLASS_LOGICAL #6949

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dalcinl
Copy link
Contributor

@dalcinl dalcinl commented Mar 19, 2024

Pull Request Description

Possible additions from the upcoming MPI 4.2 standard.

Author Checklist

  • Provide Description
    Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
  • Commits Follow Good Practice
    Commits are self-contained and do not do two things at once.
    Commit message is of the form: module: short description
    Commit message explains what's in the commit.
  • Passes All Tests
    Whitespace checker. Warnings test. Additional tests via comments.
  • Contribution Agreement
    For non-Argonne authors, check contribution agreement.
    If necessary, request an explicit comment from your companies PR approval manager.

@dalcinl
Copy link
Contributor Author

dalcinl commented Mar 25, 2024

@hzhou I'm waiting for some feedback on the C implementation. Fortran is still a TODO, I'm not sure what's the best way to implement support for MPIX_* constants.

Copy link
Contributor

@hzhou hzhou left a comment

Choose a reason for hiding this comment

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

What will be the difference between MPI_INTEGER* and MPI_LOGICAL* ?

@dalcinl
Copy link
Contributor Author

dalcinl commented Mar 25, 2024

What will be the difference between MPI_INTEGER* and MPI_LOGICAL* ?

I'm not sure I understand your question! What if I ask you what's the difference between logical*n and integer*n in Fortran? Beyond logical*1, all the other types are just a wasteful way of representing a boolean, just like the plain logical type is.

MPI_LOGICAL* should only support logical reduce operations like MPI_LAND, just like MPI_LOGICAL or MPI_C_BOOL. If I got things right, but listing MPI_LOGICAL* in the appropriate macros, these operations would be automatically defined. Am I right?

@dalcinl
Copy link
Contributor Author

dalcinl commented Mar 25, 2024

In the last Forum meeting, I made the point that the really important type is logical*1, because that type is the Fortran fixed-size equivalent to logical(kind=C_BOOL) interoperable with C99 bool/_Bool type.

@hzhou
Copy link
Contributor

hzhou commented Mar 25, 2024

What if we simply #define MPI_LOGICAL1 MPI_INTEGER1, and so on? Everything will just work, right?

@dalcinl
Copy link
Contributor Author

dalcinl commented Mar 25, 2024

That's not the way MPI datatypes are specified in the MPI standard. For example, MPI_INT32_T is not a preprocessor alias for MPI_INT or MPI_INTEGER and alias for MPI_INTEGER4 (assuming the sizes match).
The MPI standard is VERY explicit about datatypes aliases, there are only a couple (MPI_LONG_LONG_INT, MPI_C_COMPLEX).
MPI_LOGICAL is not an alias for MPI_INTEGER. Likewise, by the symmetry argument, the fixed-size logical types should not be aliases for the fixed-size integral types. MPI_Type_get_name(MPI_LOGICAL1, name) would return MPI_INTEGER1 . In Fortran, integer types are not even valid in logical context like they are in C (despite gfortran allows using integrals in logical contexts as an extension).

Now, you may very well oppose the proposal to add MPI_LOGICAL*n to MPI. If you plan to vote against such addition, then this PR should be put on hold. I don't think this is the proper place to discuss about it, that should happen here: mpi-forum/mpi-issues#699 and https://github.com/mpi-forum/mpi-standard/pull/963.

PS: Open MPI have had support for MPI_LOGICAL{1,2,4,8} since forever, not even in the MPIX_* namespace. I've submitted a PR to add MPI_LOGICAL16.

@hzhou
Copy link
Contributor

hzhou commented Mar 25, 2024

I am arguing what is the necessity of adding these Fortran logical types. Fortran users can use the INTEGER equivalent to work around if needed. But I suspect there is no real need from Frotran users. I am just a bit annoyed by these ideological complexity pushed down to implementors. I would be much happier with knowing that our effort is serving some real users.

Anyway, if MPI-Forum added these to the standards, we will support them, but I would wait until the official standard is published.

@dalcinl
Copy link
Contributor Author

dalcinl commented Mar 25, 2024

As I said before, this is not the proper place to discuss the fundamentals of additions. These "ideological complexities" as you named them were added to the Fortran 2008 standard in the first place. If supporting the standard full range of Fortran types is not a compelling enough argument for you, then there is no point on keep arguing. If you tell me that logical(kind=8) is a nonsense, I have zero argument to contradict your statement, but like it or not, the damn thing is supported by most Fortran compilers, probably just because for every integer type they have a matching-size logical type. On a personal note, the fact that MPI does not have MPI_LOGICAL1 has always pissed me off, because there is not Fortran MPI datatype that is equivalent to logical(kind=C_BOOL). In Fortran, INTEGER is not a replacement for LOGICAL. The Fortran standard does not even define what the actual bit representation of .TRUE. and .FALSE..

Anyway, if MPI-Forum added these to the standards, we will support them, but I would wait until the official standard is published.

OK, fair enough... you are definitely entitled to allow any MPIX_ extensions you like and agree with, and boicot the ones you do not like. Please close this PR and let's move on.

@dalcinl
Copy link
Contributor Author

dalcinl commented Mar 28, 2024

I would like to put a formal request to the MPICH community at large to move this PR forward and make the proposed feature available now rather than waiting for the upcoming MPI standard revision. I'll state below my rationale for this request.

First, just in case, a clarification: this feature has nothing to do with the in-discussion MPI ABI effort or any of the controversies about it.

Since its inception, MPI has supported fixed-size MPI_INTEGER{N} types to match the (nonstandard?) Fortran language INTEGER*N types. Fortran 2008 normalized this stuff via the iso_fortran_env module, adding fixed-size kind parameters to support integer(kind=INT{8,16,32,64,128}), with the list of valid integer kinds are available in the integer_kinds constant array. The Fortran committee also added support for fixed-size logical(kind=K) types, and the list of valid kinds is available in logical_kinds constant array. The MPI standard predates the publication of the Fortran 2008 standard. When the MPI Forum added support for Fortran 2008 and use mpi_f08, unfortunately they forgot to update the list of MPI datatypes to preserve the symmetry of the MPI<->Fortran type system. mpi-forum/mpi-issues#699 is an attempt to rectify this long-overdue update.

Beside the "purity" argument above about preserving symmetry in the type system, there are practical use cases that justify this addition. I think no one will object C-Fortran interoperability is very important. C99 (finally!) added a _Bool type, available as simply bool if #include <stdbool.h> and supported in MPI with MPI_C_BOOL. The corresponding C-interoperable Fortran type via iso_c_bindings is logical(kind=C_BOOL). If a user wants a predefined MPI datatype in the Fortran side to operate with scalars and arrays of such logical type, currently there is no clean way to do it. One of the natural ways to do it would be to call MPI_Type_match_size(MPI_TYPECLASS_LOGICAL, sizeof(X), datatype). This routine is expected to return in datatype a predefined datatype that should be MPI_LOGICAL1. Therefore, type matching for logical types is practically useful, and MPI_LOGICAL1 is a must. Then, if MPI has to support, MPI_LOGICAL1, supporting the others may not be a big deal (this is admittedly uninformed opinion, this PR has not yet addressed the Fortran side of things).

Finally, I would like to add that I believe this feature will eventually be accepted by the Forum. The type-system symmetry and and interoperability arguments discussed above are hard to beat down. No one has so far raised any concerns about this addition in the MPI Forum issue, nor in the the last Forum meeting I attended.

Therefore, I ask the MPICH community at large to consider my request to add the features proposed in this PR as an MPIX_ extension as soon as this PR is finished and all any remaining bits are sorted out.

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