mpi-forum / mpi-issues

Tickets for the MPI Forum
http://www.mpi-forum.org/
67 stars 8 forks source link

MPI_Comm_disconnect is not stated to delete attributes #687

Closed jeffhammond closed 1 year ago

jeffhammond commented 1 year ago

Problem

MPI_Comm_disconnect does not appear in the text below (§7.7), which means that attribute state will be leaked if the user is counting on their own comm_delete_attr_fn to do something of that nature.

Analogous to comm_copy_attr_fn is a callback deletion function, defined as follows. The comm_delete_attr_fn function is invoked when a communicator is deleted by MPI_COMM_FREE or when a call is made explicitly to MPI_COMM_DELETE_ATTR. comm_delete_attr_fn should be of type MPI_Comm_delete_attr_function.

This function is called by MPI_COMM_FREE, MPI_COMM_DELETE_ATTR, and MPI_COMM_SET_ATTR to do whatever is needed to remove an attribute. The function returns MPI_SUCCESS on success and an error code on failure (in which case MPI_COMM_FREE will fail).

Proposal

Add MPI_Comm_disconnect to the above, since it is equivalent to MPI_Comm_free from the perspective of attributes.

Changes to the Text

The trivial change is shown in italics below:

Analogous to comm_copy_attr_fn is a callback deletion function, defined as follows. The comm_delete_attr_fn function is invoked when a communicator is deleted by MPI_COMM_FREE, _MPI_COMMDISCONNECT or when a call is made explicitly to MPI_COMM_DELETE_ATTR. comm_delete_attr_fn should be of type MPI_Comm_delete_attr_function.

This function is called by MPI_COMM_FREE, _MPI_COMMDISCONNECT, MPI_COMM_DELETE_ATTR, and MPI_COMM_SET_ATTR to do whatever is needed to remove an attribute. The function returns MPI_SUCCESS on success and an error code on failure (in which case MPI_COMM_FREE will fail).

Impact on Implementations

MPICH already does the right thing since both MPI_Comm_free and MPI_Comm_disconnect both end up in MPIR_Comm_delete_internal, which invokes the appropriate callback.

Impact on Users

It is unlikely that users are reading the standard this carefully :-)

References and Pull Requests

jeffhammond commented 1 year ago

I propose that this is a CC change because it should not impact anyone and is likely the result from an editorial oversight.

wgropp commented 1 year ago

I agree with Jeff, and this is another example of the danger of ad hoc lists in the standard.

wesbland commented 1 year ago

Assigning this to the Communicator chapter committee members since it's a CC change.

GuillaumeMercier commented 1 year ago

I agree with Jeff, and this is another example of the danger of ad hoc lists in the standard.

I have to check with Martin R. because I think he has a way to generate lists.

jaegerj commented 1 year ago

We quickly discussed this during the last meeting. It could be done, but it is not ready yet if I recall correctly. I think it depends on which information is needed to generate the required list, and if this information is already existing in the python representation.