Skip to content

Conversation

@jsquyres
Copy link
Member

@jsquyres jsquyres commented Feb 22, 2022

Ensure to check for MPI_DATATYPE_NULL before calling
PMPI_Type_get_name(). As of MPI-4.0, there's still at least some
disagreement as to whether MPI_DATATYPE_NULL is a valid input
parameter for MPI_TYPE_GET_NAME. While that discussion is ongoing,
fix up libompitrace to do what is guaranteed to work.

Signed-off-by: Jeff Squyres [email protected]

Refs #10028, #10029, mpi-forum/mpi-issues#544, https://github.com/mpi-forum/mpi-standard/pull/652

Ensure to check for MPI_DATATYPE_NULL before calling
PMPI_Type_get_name().  As of MPI-4.0, there's still at least some
disagreement as to whether MPI_DATATYPE_NULL is a valid input
parameter for MPI_TYPE_GET_NAME.  While that discussion is ongoing,
fix up libompitrace to do what is guaranteed to work.

Signed-off-by: Jeff Squyres <[email protected]>
@bosilca
Copy link
Member

bosilca commented Feb 22, 2022

I don't think this is the right approach, and if we merge this PR we will be stuck for years with an incorrect code.

@jsquyres
Copy link
Member Author

Can you clarify?

  1. What do you think is wrong about this approach?
  2. What do you think is incorrect code?

@jsquyres
Copy link
Member Author

bot:ompi:retest

@bosilca
Copy link
Member

bosilca commented Feb 23, 2022

This patch correctly fixes the MPI_DATATYPE_NULL use in opmpitrace, with disregard on how we handle the MPI_DATATYPE_NULL in MPI_Type_get_name, and without dependency on what the forum decides to do with the *_NULL objects. We need to back-ported this to all versions we still care about.

Copy link
Member

@bosilca bosilca left a comment

Choose a reason for hiding this comment

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

All good, but this is the reason why we should be more user-friendly and handle the MPI_*_NULL get_name inside the MPI library.

@jsquyres
Copy link
Member Author

@bosilca and I chatted about this on the phone this morning. This PR makes libompitrace valid in all cases, regardless of whether the MPI Fourm accepts mpi-forum/mpi-issues#544 or not.

@jsquyres jsquyres merged commit dbb12dd into open-mpi:master Feb 23, 2022
@jsquyres jsquyres deleted the pr/libompitrace-and-datatype-null branch February 23, 2022 16:31
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.

2 participants