Re: [Boost-mpi] Thread API

Subject: Re: [Boost-mpi] Thread API
From: Matthias Troyer (troyer_at_[hidden])
Date: 2012-11-28 11:35:22

On Nov 28, 2012, at 11:21 AM, Hal Finkel <hfinkel_at_[hidden]> wrote:

> ----- Original Message -----
>> From: "Matthias Troyer" <troyer_at_[hidden]>
>> To: "Discussion of Boost.MPI development" <boost-mpi_at_[hidden]>
>> 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 <hfinkel_at_[hidden]> 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).
>> 
>> 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.

>> 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.
> 

I agree.

One final change suggestion: should introduce macros BOOST_HAVE_MPI2 and BOOST_HAVE_MPI3 to conditionally include MPI-2 and MPI-3 features.

Matthias