Hi all, Just as a reminder - our next MPI-3 tools WG TelCon is scheduled for Monday morning at the usual time (8am PST, 11am EST, 5pm MZ). The main agenda point is to discuss the latest MPIT draft. The access link is on the Wiki and as follows: https://cisco.webex.com/cisco/j.php?ED=155386812&UID=0&PW=NMTJhNWQxOTdj&RT=M... Thanks to Kathryn and Dave for the feedback - comments and replies are below. I also made matching changes to the text and committed it to the SVN repo (in the new structure!). I also uploaded it to the Wiki at: https://svn.mpi-forum.org/trac/mpi-forum-web/wiki/MPI3Tools/mpit and attached it here. Talk to you on Monday, Martin From Kathryn: All small fixed from text file added.
1) I believe we decided that all of MPIT is available before MPI_Init, just that some variables may not exist or have valid values before MPI_Init (e.g. page 16, line 20). However, on page 1 line 48, it sounds to me like only MPIT_Init and MPIT_Finalize can be called outside of MPI_Init and MPI_Finalize. We should make sure that we clearly specify what is and is not allowed to be called outside of MPI_Init/Finalize.
Good point - that's not how it is meant, but that is how it reads. I changed the text to reflect this.
2) On page 18, the perfvar start and stop routines return an error if not all variables are started when MPIT_PERFVAR_ALL_HANDLES is specified. I think that we decided that the routines would return MPIT_ERR_NOSTARTSTOP and that the user was responsible for looping through the variables starting or stopping them individually to find out where the problem was. If this is correct, we should explicitly state it. (This is also true for reset.)
I added a comment to each case.
3) At the start of the categories section, I think a brief example of variable categories and category hierarchies would go a long way. Without this intuition, I think the section will be somewhat confusing. Also, why does category_get_info return individual values for number of control vars, perf vars, and categories if the call to category_get_contents gives you no control over what is returned, just gives you "kinds" to sort out what is what. Maybe it should just return a content_count instead of the individual counts?
Let's leave this for discussion on Monday - hopefully Dave can join us for this. From Dave:
section 1.2.2: can we cut "MPI_" from all of the "MPIT_MPI_RESOURCE_TYPE_BLAH" constants and related functions? These names seem unnecessarily wordy.
I am not a big fan of the names either, but the idea was to indicate that these are MPIT constants, but MPI resources. I can go either way here, but I would like to hear from the rest of the group before I change it throughout the document.
page 4: MPIT_MPI_RESOURCE_COMMUNICATOR takes an MPI_Comm as an IN parameter, but the binding has a pointer as the type. Same goes for all of the other similar functions on this page.
The idea is to allow simple pointer conversion implementations, which I would expect many would want to do. If we remove the "*" this would no longer be possible. The IN should indicate, though, that the argument is not being modified. Is there a better way to do this? I guess, this is mainly a question for the implementors.
page 4, line 33: "MPI_" should be "MPI_Group"
fixed.
page 5, line 29: "MPIT_MPI_Resource*" seems to be in the wrong typeface. This is a C language type, so it should be typewriter text.
It has the right macro - I'll have to double check this with the rest of the standard.
General comment about this section and functionality: Since we assume only C bindings, can we just replace all of these functions with a single function? Something like:
MPIT_Get_resource(void *handle_ptr, int resource_type, MPIT_MPI_Resource *out_resource);
and a call to it would look something like this: -----8<---- int type; int index = 0; MPIT_MPI_Resource resource; MPIT_Controlvar_handle handle; MPI_Comm comm = MPI_COMM_SELF; MPIT_Controlvar_get_info(index, ..., &type, ...); if (type == MPIT_MPI_RESOURCE_TYPE_COMMUNICATOR) { MPIT_Get_resource(&comm, type, &resource); MPIT_Controlvar_handle_allocate(index, resource, &handle); /* read, write, etc here using handle */ } -----8<----
That's pretty close to what I had last time, but I had the feeling that the majority didn't like that and instead wanted a type safe option as the current one. Any other comments on this one? Both options would work and each has their pluses and minuses.
I think the string arguments section (1.2.3) could use just a little polishing to make its behavior crystal clear, but it's much better than it was. Also, "deposits"-->"writes".
I replaced deposits with writes - besides that, do you have any concrete suggestions or do you think anything particular may be unclear? I have seen this now so often that I have a hard time telling.
I'll admit I didn't have time to thoroughly read the perfvar section, so there might still be issues there.
As for the category section, it looks basically OK to me. I thought that we decided at the last call that categories would be flat and wouldn't contain other categories. If you insist on pursuing this approach further, you'll need more text to explain that categories can't contain each other or themselves cyclically.
I thought we decided for including them, but perhaps this was just my preference. I still think it can't hurt and gives us that little extra flexibility. I agree, though, if we go that route we need to make these restrictions clear. I added some text for this purpose.
A general language comment: you should basically do a search and replace of "have to" or "has to" with "must" throughout the document.
fixed ________________________________________________________________________ Martin Schulz, [email protected], http://people.llnl.gov/schulzm CASC @ Lawrence Livermore National Laboratory, Livermore, USA