----- Original Message ----- > From: "Matthias Troyer" <[email protected]> > To: "Discussion of Boost.MPI development" <[email protected]> > Sent: Wednesday, November 28, 2012 9:41:43 AM > Subject: Re: [Boost-mpi] Thread API > > > On Nov 28, 2012, at 10:21 AM, Hal Finkel <[email protected]> wrote: > > > ----- Original Message ----- > >> From: "Matthias Troyer" <[email protected]> > >> To: "alain miniussi" <[email protected]>, "Discussion of > >> Boost.MPI development" <[email protected]> > >> 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 > >> <[email protected]> > >> 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). > > Thank you, that was one of the points I also wanted to raise. > > Another comment was that I would prefer threading::multiple over > mt::multiple - would you mind changing the namespace?
I also like this better. > > > > > > + 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). > > A very good point. One might think about a constructor that has the > more relaxed semantics but we might want to specify this explicitly, > like on > > boost::mpi::environment env(argc, argv, > threading::attempt(threading::multiple)); That seems reasonable. > > or similar. > > I have another small worry. We have two similar constructors now: > > environment(int& argc, char** &argv, bool abort_on_exception = > true); > environment(int& argc, char** &argv, mt::level mt_level, bool > abort_on_exception = true); > > Should we worry about legacy codes that passes an int as third > argument? We could avoid that by making the threading level a class > instead of an enum, or a strongly typed enum in C++11. I'm not terribly worried about that; having the enum constants interchangeable with the MPI constants seems convenient. -Hal > > Matthias > > > > 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 > >> [email protected] > >> http://lists.boost.org/mailman/listinfo.cgi/boost-mpi > >> > > > > -- > > Hal Finkel > > Postdoctoral Appointee > > Leadership Computing Facility > > Argonne National Laboratory > > _______________________________________________ > > Boost-mpi mailing list > > [email protected] > > http://lists.boost.org/mailman/listinfo.cgi/boost-mpi > > > > _______________________________________________ > Boost-mpi mailing list > [email protected] > http://lists.boost.org/mailman/listinfo.cgi/boost-mpi > -- Hal Finkel Postdoctoral Appointee Leadership Computing Facility Argonne National Laboratory _______________________________________________ Boost-mpi mailing list [email protected] http://lists.boost.org/mailman/listinfo.cgi/boost-mpi
