Re: [Boost-mpi] Thread API

Subject: Re: [Boost-mpi] Thread API
From: Alain O Miniussi (alain.miniussi_at_[hidden])
Date: 2012-11-29 06:06:06

On Wed, 2012-11-28 at 14:22 -0600, Hal Finkel wrote: 
> > So, to summarize, we have 3 issues:
> > 1) syntax: lower vs upper case and mt->threading.
> > 2) throw an exception if the requested level is not available
> > 3) macro to control availability of MPI[1|2|3] features

In the attached patch:
1 -> done
3 -> not done, I think those macro should be introduced separately, not
in the same commit.
2 -> done, but more precisely:
 The ctors that took one level requirement now takes a preferred and a
required level. If the required level could not be provided, an
exception is thrown from the ctor *after* MPI_Finalize is call.
This seems necessary since otherwise, MPI_Abort would get called in the
dtor and the application would quit without MPI_Finalize getting called.
 Also, that behavior introduces an error that does not mirror an MPI
error (in the MPI standard). Seems to be the first one, which explain
that I modified the exception hierarchy: We now have a base generalist
exception class (which only take a message) and an derived api_exception
that correspond to the previous exception class (and that contains all
the info related with the MPI standard error). Since the code often ask
a corresponding MPI error code, I choose to bind it to MPI_SUCCESS by
default (indicating that as far as the MPI standard is concerned, there
is no problem) api_exception systematically overrides that code.

I also added a istream>>level operator, is nothing else, it will enable
lexical_cast.

Thanks for reading !

Alain

> > 1) ok, I like it better, too :-)
> > 
> > 2) exceptions  -> if I understand, it is proposed that we consider
> > the
> > unavailability of the requested level as an error and let the flag
> > 'throw_on_error' decide what to do ?
> > A few small concerns
> >  * the MPI standard does not seems to consider this an error, so we
> >  do a
> 
> The standard does not consider it an error, but it also assumes that the caller will check the provided level. This is a bad assumption, and we must have a better option ;)
> 
> > semantic change (if I understand correctly 8.7 in MPI3)
> >  * it is not clear that we are allowed to query the available
> >  threading
> > support level (actually, 8.7 seems to indicated that we can't,
> > although
> > there is some weird text associated with the thread_level key of
> > INFO_ENV). That indicates that the user has no way of probing the
> > value
> > before initializing. So if we consider the use case of a user who
> > wants
> > to generally throw on error *and* is willing to adapt to the
> > available
> > threading level, such a change will make that impossible.
> 
> Okay, good point, and calling MPI_Init multiple times may not be safe. How about this: have the constructors take both a 'preferred' and 'required' level; call MPI_Init with the preferred level and throw the exception only if the provided level is below the required level. This has the advantage of being safe and easy, forcing the programming to think about the options, and is not functionally limiting. If a program really has requirements that cannot be expressed as a range in this way, they can always set the required level to the lowest one and fall back to explicit post-construction tests.
> 
> >  * we can easily imagine ignoring the result of "provided" level (by
> > default, open MPI only support "single", so in theory, one should not
> > use threads at all with open MPI. Multiple must be explicitly
> > specified
> > during build, and is possibly slow, and there is nothing in between)
> 
> Yes; unfortunately, this is a (somewhat common, although certainly not universal) implementation deficiency.
> 
> > - We cannot fix that with bit magic (allowing stuff like
> > threading::funneled|threadin::mandatory for example) since the
> > numerical
> > value are not specified by the standard.
> > - Adding a boolean flag would be clumsy (we already have one with
> > default value)
> > - we could allow runtime modification of the throw_on_error flag, but
> > then, it is not clear that the most common scenario (and the one
> > matching the MPI-C behavior) would be the easiest.
> > 
> > On the other hand, if the user wants to enforce the threading level:
> > mpi::environment env(threading::funneled, true);
> > assert(env.thread_level() >= threading::funneled); /* or an 'if'
> > expression, or we could add an overload to the boost mpi abort to
> > make
> > it more assert like:
> > env.assert(env.thread_level()>=threading::funneled, "requested ...
> > blahh"); */
> > which is basically one more line.
> 
> I don't think is good because it still makes it easier to write incorrect code than it does to write correct code.
> 
> > 
> > Any thought ?
> > 
> > 3) Ok, actually the BOOST_MPI_HAS_NOARG_INITIALIZATION would fall in
> > that category (the feature is mandatory since MPI2 I think).
> > 
> > One last question: in my patch, I am still using MPI_Init instead of
> > the
> > supposedly equivalent MPI_Init(..single..) (keept for backward
> > compatibility) I propose to only use MPI_Init_thread in the
> > implementation.
> 
> This is fine with me, so long as we're not keeping compatibility with MPI 1 implementations. Are we?
> 
> Thanks again,
> Hal
> 
> > 
> > Thanks for reading !
> > 
> > Alain
> > 
> > 
> >  
> > 
> >    
> > 
> > On Wed, 2012-11-28 at 09:21 -0600, Hal Finkel wrote:
> > > ----- Original Message -----
> > > > From: "Matthias Troyer" <troyer_at_[hidden]>
> > > > To: "alain miniussi" <alain.miniussi_at_oca.eu>, "Discussion of
> > > > Boost.MPI development" <boost-mpi_at_[hidden]>
> > > > Sent: Wednesday, November 28, 2012 8:25:18 AM
> > > > Subject: Re: [Boost-mpi] Thread API
> > > > 
> > > > Hi Alain,
> > > > 
> > > > 
> > > > On Nov 27, 2012, at 5:11 PM, Alain O Miniussi
> > > > <alain.miniussi_at_oca.eu>
> > > > wrote:
> > > > 
> > > > > Hi,
> > > > > 
> > > > > I have a tentative patch that is supposed to  provide binding
> > > > > to
> > > > > the MPI
> > > > > thread API. What is the best way to have it reviewed ?
> > > > 
> > > > Thank you for your work. I have a few stylistic comments - shall
> > > > I
> > > > send them to you off-list, or shall we discuss them on our new
> > > > mailing list?
> > > 
> > > If you don't mind, I'd prefer on-list reviews (even for stylistic
> > > matters). That way we can all benefit from your advice.
> > > 
> > > I also have a few comments:
> > > 
> > > +enum level {
> > > +  /** Only one thread will execute.
> > > +   */
> > > +  SINGLE     = MPI_THREAD_SINGLE,
> > > +  /** Only main thread will do MPI calls.
> > > 
> > > I think that we should use lower-case enum constant names (single
> > > instead of SINGLE).
> > > 
> > > +  mt::level mt_level = mt::MULTIPLE;
> > > +  boost::mpi::environment env(argc, argv, mt_level);
> > > +  mt::level provided = env.thread_level();
> > > +  std::cout << "Asked:" << mt_level << ", provided: " << provided
> > > << '\n';
> > > +  BOOST_CHECK((provided >= mt::SINGLE && provided <=
> > > mt::MULTIPLE));
> > > 
> > > I have a lot of code that looks like this ;) -- but I think that it
> > > could be greatly simplified. How about if the environment
> > > constructor throws an exception if the provided level is less than
> > > the requested level. This way, in the common case where the
> > > program has no fall-back provisions, we don't accidentally
> > > continue under an invalid environment (many programs don't
> > > explicitly check the provided level -- and this obviously leads to
> > > odd behavior and crashes).
> > > 
> > > Thanks for working on this!
> > > 
> > >  -Hal
> > > 
> > > > 
> > > > > It might be a ticket
> > > > > buthttps://svn.boost.org/trac/boost/report/15the
> > > > > referenced maintainer is Doug
> > > > > (https://svn.boost.org/trac/boost/report/15)
> > > > > and, probably since I have no userid in the tracker, I won't be
> > > > > able to
> > > > > change it.
> > > > 
> > > > I'll try to find out how to change it. Doug does not have time
> > > > anymore.
> > > > 
> > > > > Just in case, there is a patch in attachment (against rev 81596
> > > > > of
> > > > > trunk).
> > > > 
> > > > 
> > > > Thank you!
> > > > 
> > > > Matthias
> > > > 
> > > > _______________________________________________
> > > > Boost-mpi mailing list
> > > > Boost-mpi_at_[hidden]
> > > > http://lists.boost.org/mailman/listinfo.cgi/boost-mpi
> > > > 
> > > 
> > 
> > 
> > 
> 

-- 
Alain Miniussi
Responsable Tech. Centre de Calcul Haute Performance
Obs. de la Côte d'Azur |Mont Gros: [phone]
BP 4229                |Sophia :   [phone]
06304 Nice Cedex 4     |https://crimson.oca.eu