The following minor issues came up during my 4.0 RC review. I discussed them with @mahermanns -- we decided that they weren't critical for 4.0, but they should probably be brought up in the Tools WG for further discussion (e.g., maybe target some updates for 4.1 or something).
[ ] p756 27-29: "It also has no effect on..." This sentence is a tautology -- it says "MPI implementations ignore hints that they ignore." I think you're trying to say that passing in values for hints that MPI implementations ignore will (still) be ignored in MPI_T_EVENT_CALLBACK_SET_INFO. But perhaps this could be phrased a bit better / worded so that it's not a tautology? Or if there's no way to not make this a tautology, perhaps this concept is suitable for an Advice to Users...?
[ ] p757 4-5: "To stop ..." this implies that the only way to stop raising events is to call HANDLE_FREE. But a user could also set all the callbacks on a given registration to NULL. Yes, that's probably foolish, but it's permitted. The wording should be changed on this sentence to not imply that calling HANDLE_FREE is the only way to stop raising events.
[ ] p758 24: the count argument in the callback function was not embiggened. I think that there was a lot of discussion about this -- the idea being that if you overflow, your app has other problems. I'm not sure that I agree with this sentiment -- perhaps your app just got busy and wasn't able to check for events in a long period of time and the events unexpectedly became very frequent. I think there's valid scenarios where an overflow of the count param can happen. Regardless, the point here is that something should be said about overflow. Perhaps MPI should just guarantee that overflow will never happen; if too many events are dropped, count will stay at INT_MAX until it is reset by the next callback, or somesuch. That way, at least an application would at least be able to categorically know "I missed a lot of events" (vs. being called back with an overflowed count that now has value of 2, and the app thinks "Oh, I only missed 2 events", when it had really missed 2^32+2 events).
[ ] p758 39: the callback described in this sentence is the main callback (vs. the dropped handler callback). It's not necessary to specify that it's the main callback in this sentence (because only the main callback has the user context pointer), but it would read slightly better if you specify that it's the main callback because the text had just described the dropped handler callback.
[ ] p761 36: Suggestion: "processing of events on the tool side, when required, while retaining..." --> remove ", when required,".
[ ] p761 42: Suggestion: remove "between various variables".
[ ] p764: MPI_T_CATEGORY_GET_CVARS has a 2-sentence description. It is then followed by MPI_T_CATEGORY_GET_PVARS, MPI_T_CATEGORY_GET_EVENTS, and MPI_T_CATEGORY_GET_CATEGORIES, each with very similar 2-line descriptions. However, the detailed description of the parameters and behavior of this function does not occur until p766 starting on line 19. For readability, I think that this description should be moved up to be under the first function (MPI_T_CATEGORY_GET_CVARS), and then the subsequent functions should say "this function behaves similarly to MPI_T_CATEGORY_GET_CVARS..." (or something like that). It's very confusing to read the text as it stands because you don't get any description of the function until 2 pages later.
[ ] Also part of MPI_T_CATEGORY_GET_CVARS and friends: it should be stated how the user knows what value of len to use (i.e., cite the corresponding GET function where you can get the proper length).
[ ] p766 34-35: Suggestion: "...was completed successfully or was aborted" --> "...was completed successfully or not."
The word "aborted" has drastic connotations in MPI.
[ ] p766 35: Suggestion: "not completing the routine" --> "the error".
[ ] p766 37-38: It is not always true that passing an invalid parameter results in undefined behavior. You should double check with what the new error handling stuff is in 4.0, and don't forget that at least some MPI_T routines are defined to return INVALID_HANDLE, INVALID_NAME, ...etc.
[ ] p768 22-23, 32: Suggestion: Everywhere you have "session" change it to "tool session" (to avoid confusion with MPI Sessions).
The following minor issues came up during my 4.0 RC review. I discussed them with @mahermanns -- we decided that they weren't critical for 4.0, but they should probably be brought up in the Tools WG for further discussion (e.g., maybe target some updates for 4.1 or something).
All page numbers are relative to the Nov 2020 4.0 RC document, attached here for reference
count
argument in the callback function was not embiggened. I think that there was a lot of discussion about this -- the idea being that if you overflow, your app has other problems. I'm not sure that I agree with this sentiment -- perhaps your app just got busy and wasn't able to check for events in a long period of time and the events unexpectedly became very frequent. I think there's valid scenarios where an overflow of thecount
param can happen. Regardless, the point here is that something should be said about overflow. Perhaps MPI should just guarantee that overflow will never happen; if too many events are dropped,count
will stay atINT_MAX
until it is reset by the next callback, or somesuch. That way, at least an application would at least be able to categorically know "I missed a lot of events" (vs. being called back with an overflowedcount
that now has value of2
, and the app thinks "Oh, I only missed 2 events", when it had really missed 2^32+2 events).len
to use (i.e., cite the correspondingGET
function where you can get the proper length).Enjoy!