-
Notifications
You must be signed in to change notification settings - Fork 890
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 new status_set/get functions #13125
mpi: add new status_set/get functions #13125
Conversation
We should probably just add the Fortran bindings in #12226 |
89977a8
to
e61248d
Compare
@jsquyres do we need to also add man-pages / documentation for these functions? |
MPI4.1 introduced a few new MPI_Status_get/set functions. This PR is the C API implementations of the functions. Signed-off-by: Edgar Gabriel <Edgar.Gabriel@amd.com>
27321d0
to
40e0021
Compare
Added a second commit to this PR which adds the man-pages for the new functions. |
Yes, please. We have man pages for all public MPI API calls.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left comments for 2 of the man pages, but the same issues exists in all the others.
|
||
INPUT/OUTPUT PARAMETER | ||
---------------------- | ||
* ``status``: Status with which to retrieve error (status). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* ``status``: Status with which to retrieve error (status). | |
* ``status``: Status from which to retrieve the error (status). |
INTEGER, OPTIONAL, INTENT(OUT) :: ierror | ||
|
||
|
||
INPUT/OUTPUT PARAMETER |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the status inout ? It should be read only for get.
|
||
USE mpi_f08 | ||
MPI_Status_get_error(status, error, ierror) | ||
TYPE(MPI_Status), INTENT(INOUT) :: status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TYPE(MPI_Status), INTENT(INOUT) :: status | |
TYPE(MPI_Status), INTENT(IN) :: status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bosilca thank you, yes, the INOUT is correct for the _set_error function, but not for the _get_error. I've updated all _set functions to apply this change
INTEGER, OPTIONAL, INTENT(OUT) :: ierror | ||
|
||
|
||
INPUT/OUTPUT PARAMETER |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
INPUT/OUTPUT PARAMETER | |
INPUTPARAMETER |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment as above (i.e. difference between _set and _get functions)
DESCRIPTION | ||
----------- | ||
|
||
Returns in error the MPI_ERROR field in the status object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returns in error the MPI_ERROR field in the status object. | |
Returns in error the MPI_ERROR field from the status object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also same as above. "from" has to be used for the _get, functions, "in" for the _set functions.
INTEGER, OPTIONAL, INTENT(OUT) :: ierror | ||
|
||
|
||
INPUT/OUTPUT PARAMETER |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
INPUT/OUTPUT PARAMETER | |
INPUTPARAMETER |
|
||
USE mpi_f08 | ||
MPI_Status_get_source(status, source, ierror) | ||
TYPE(MPI_Status), INTENT(INOUT) :: status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TYPE(MPI_Status), INTENT(INOUT) :: status | |
TYPE(MPI_Status), INTENT(IN) :: status |
40e0021
to
b726671
Compare
Signed-off-by: Edgar Gabriel <Edgar.Gabriel@amd.com>
b726671
to
2608b02
Compare
I'm really curious how a user is supposed to properly use these new API and obtain a valid status ? Are they supposed to set it to zero before calling any of these functions ? Why would that be valid ? |
@bosilca, thank you first of all for your reviews, you caught a number of issues! I am not entirely sure to be honest, my understanding is that the functions are intended to improve the support of MPI from other languages. (In fact, despite of having attended basically all MPI meetings for 4.1, I don't have a good recollection on these functions, which I find surprising). I just tried to tick off another box for the MPI 4.1. compliance by implementing these functions. |
These functions are not used to get a status. They are meant to extract the fields of a status received from test/wait in languages that cannot deal directly with C struct fields (like Python). |
That was absolutely not my question. My question was about the set part of the API. |
There is no such thing as a "valid status" in MPI. Different operations touch different fields and leave the rest untouched (source/target/tag for non-persistent recv, error for test/wait_[some,all] in case of an error. The setter functions are relevant for generalized requests so the application sets whatever fields it needs and leaves the rest undefined. |
Of course there is a valid status, most operations start from the empty status and fill it up. It is well defined for all types of operations. But here we allow libraries to manipulate statuses without enforcing them to start from a sane value. Basically the only way to use such a status is to force the user is to always know exactly what fields have been set and never access any other fields from the status. Quite a high bar. |
MPI4.1 introduced a few new MPI_Status_get/set functions. This PR is the C API implementations of the functions.
Fixes issue #12073
Note: this is just the C-API, I need potentially help for the Fortran API of these functions.