Attached is part of a note Jeff sent me (The C++ police at Notre
Dame). He warned me it is long and complex but I wanted people to
know what is going on. Due to his comments, I propose the following
ideas for comment:
- Change the names of the GR functions because the names are hold
overs from when we had a separate type of request. The names would
now be MPI_REQUEST_MARK_COMPLETE AND MPI_REQUEST_START_GR. Other
ideas?
- Change the C++ GR callbacks to be in the class REQUEST. Thus, they
would be like MPI::Request::Query_fn, etc.
- Does anyone think changing the strings in comm name and error string
to be char[] instead of char * is a bad idea. I know we discussed it
but don't recall the reasons right now.
- Change the thread names to:
Old New
MPI_THREAD_INIT -> MPI_INIT_THREAD
MPI_THREAD_INITIALIZED -> MPI_IS_THREAD_INITIALIZED
MPI_THREAD_MAIN -> MPI_IS_THREAD_MAIN
MPI_THREAD_QUERY -> MPI_QUERY_THREAD
- Change the keyval create and free functions to be like:
Old New
MPI_COMM_CREATE_KEYVAL -> MPI_CREATE_COMM_KEYVAL
MPI_COMM_FREE_KEYVAL -> MPI_FREE_COMM_KEYVAL
Seems like we should do the same for type and win too. I see the
reasoning here but don't like the difference in these names vs the
other related functions.
The details are below.
Steve
> - The generalized request routines. I see that you have named them
> MPI_GR_*. This isn't really consistent, because there is no MPI_Gr
> opaque object. The goal is to name functions MPI_object_action_subset,
> where MPI_object is a valid MPI opaque object. If the function affects
> the MPI environment, it is assumed that the object is the implicit
> "global" or "environment" object, and is just dropped (e.g.
> MPI_ATTACH_BUFFER).
>
> My problem is that your GR functions have an MPI_Request in the argument
> list, and therefore should be in the MPI::Request class (I think -- more
> on this in a minute). Additionally, your current C++ binding is
> MPI::Gr::Start(...). I do not see justification for making an
> additional class called MPI::Gr, especially if it is not a real MPI
> opaque object.
>
> My $0.02 is that it should be in the MPI::Request class, but this is
> also uncertain. Making it in the MPI::Request class would make your
> only OUT variable be the C++ controlling object, which is what most
> people didn't like about Andy's and my original C++ proposal. However,
> this may be a special case. I have send the GR functions off to Bill
> Saphir (the chief opposer of the original C++ proposal) and Eric Salo
> (my cohort in the naming scheme).
>
> The long and the short of it is, the most obvious way to rename these
> functions would be MPI_REQUEST_START_GR and
> MPI_REQUEST_MARK_COMPLETE_GR, which would translate nicely into the C++
> MPI::Request::Start_gr() and MPI::Request::Mark_complete_gr(). But this
> is up in the air because of the OUT problem. Any thoughts or comments
> on this?
>
> - Regardless of the outcome of the above point, I would like to change
> the scope of the supporting functions of the generalized start request
> function. Right now, the C++ bindings are MPI::Query_fn, MPI::Free_fn,
> and MPI::Cancel_fn. I would like to put them all in the MPI::Request
> class so that a) we don't have name clashes in the future (i.e. if
> someone else wants to use the name "FREE_FN"), and b) the C++ scoping
> facilities were created just for such purposes. :-)
>
> So I'd like to change them to: MPI::Request::Query_fn,
> MPI::Request::Free_fn, and MPI::Request::Cancel_fn. This is not a big
> deal, since I'm only asking for the C++ names to change. But, on the
> other hand, to prevent future name clashes, it would be nice to change
> the C and Fortran names to MPI_REQUEST_QUERY_FN, et al. -- it would also
> be consistent with the who attribute caching function typedef scheme at
> the end of the chapter.
>
> - In the naming communicators section, is there a reason that you used
> "char *" instead of "char []"? With "char []", it would seem that you
> have the following advantages:
>
> - Consistency with the name arguments to the MPI_PORT_* calls in the
> dynamic process chapter
> - The IN parameter names can be marked "const" (const char name[])
> - No confusing C++ binding in MPI_COMM_GET_NAME -- currently it is
> "char *& name", which is gross. It would change to "char name[]"
> - Since you already defined a max size, MPI::MAX_NAME_STRING, users
> will have to malloc this space anyway, so why not just have them use
> arrays? (The PORT calls in the dynamic chapter don't have a MAX size
> yet, but Bill promises to put them in)
>
> Also, in the MPI_COMM_GET_NAME, why is a pointer returned? It would
> seem nicer to copy the name out rather than return a pointer to the real
> McCoy. It would have the following advantages:
>
> - Buffer size would not be a problem -- the user can malloc
> MPI::MAX_NAME_STRING and the name would be guaranteed to fit in there
> - It's primarily a debugging call (according to the text), so the added
> cost of making a copy should be negligable/ignorable because the user is
> debugging anyway
> - Huge chunks of text can be eliminated about how it is erroneous to
> change the value of the name as result of the return from
> MPI_COMM_GET_NAME.
> - Not giving the user to make stupid mistakes by changing the name with
> the result of MPI_COMM_GET_NAME is arguably a Good Thing.
>
> What do you think?
>
> - I bring up the same "char []" vs "char *" argument in
> MPI_ADD_ERROR_STRING
>
> - I have a naming problem with the MPI_THREAD_* calls. It's mainly the
> same problem as I discussed above -- there is no MPI_Thread object.
> They are really all affecting the global MPI environment, so I propose
> the following name changes:
>
> Old New
> MPI_THREAD_INIT -> MPI_INIT_THREAD
> MPI_THREAD_INITIALIZED -> MPI_IS_THREAD_INITIALIZED
> MPI_THREAD_MAIN -> MPI_IS_THREAD_MAIN
> MPI_THREAD_QUERY -> MPI_QUERY_THREAD
>
> The new names are all members of the implicit "global" MPI object.
>
> - There is a bit of inconsistency in the attribute caching functions --
> it's only evident if you really look at the C++ bindings. I'll just
> discuss the MPI_COMM_* functions, but the same things apply to the
> datatype and window caching functions.
>
> There is a naming problem with the MPI_COMM_CREATE_KEYVAL and
> MPI_COMM_DELETE_KEYVAL functions -- there is no MPI_COMM in the argument
> list. This is a problem because it implies that the C++ binding should
> be MPI::Comm::Create_keyval and MPI::Comm::Delete_keyval. But the
> MPI::Comm class shouldn't be the controlling object if there isn't one
> in the arugment list. Here's an example: I'm making some keyvals:
>
> void
> foo()
> {
> MPI::Comm mycomm = MPI::COMM_WORLD.Dup(); // So mycomm is an
> MPI_COMM_DUP of MPI_COMM_WORLD
> int keyval;
>
> keyval = mycomm.Create_keyval(...); // Note that C++ functions return
> their OUT values, so the keyval is
> // returned
> // So this implies that the value in keyval is only good on mycomm.
> But not so! It's also usable
> // on MPI::COMM_WORLD. So I argue that this syntax does not clearly
> indicate what is happening
> }
>
> I think that perhaps renaming these two functions might solve the
> problem:
>
> Old New
> MPI_COMM_CREATE_KEYVAL -> MPI_CREATE_COMM_KEYVAL
> MPI_COMM_FREE_KEYVAL -> MPI_FREE_COMM_KEYVAL
>
> So both functions become members of the MPI "global" object, but are
> distinguised from the TYPE and WIN functions by their subset names.
> Additionally, I think that the typdef function names should be changed:
>
> Old New
> MPI::Comm::Copy_fn -> MPI::Comm::Copy_attr_fn
> MPI::Comm::Delete_fn -> MPI::Comm::Delete_attr_fn
>
> These new names are not necessary, but I think they are better because
> they are more specific. I guess I'm just being picky here (even more so
> than the rest of this letter :-).
>
> The MPI_COMM_PUT_ATTR and MPI_COMM_GET_ATTR functions seem fine. I have
> changes to the bindings, but I can just change those silently.