Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] RMAPS rank_file component patch and modifications for review
From: Ralph H Castain (rhc_at_[hidden])
Date: 2008-03-26 08:50:02


I would tend to echo Tim's suggestions. I note that you do lookup that opal
mca param in orte as well. I know you sent me a note about that off-list - I
apologize for not getting to it yet, but was swamped yesterday.

I think the solution suggested in #1 below is the right approach. Looking up
opal params in orte or ompi is probably not a good idea. We have had
problems in the past where params were looked up in multiple places as
people -do- sometimes change the names (ahem...).

Also, I would suggest using the macro version of verbose OPAL_OUTPUT_VERBOSE
so that it compiles out for non-debug builds - up to you. Many of us use it
as we don't need the output from optimized builds.

Other than that, I think this looks fine. I do truly appreciate the cleanup
of ompi_mpi_init.

Ralph

On 3/26/08 6:09 AM, "Tim Prins" <tprins_at_[hidden]> wrote:

> Hi Lenny,
>
> This looks good. But I have a couple of suggestions (which others may
> disagree with):
>
> 1. You register an opal mca parameter, but look it up in ompi, then call
> a opal function with the result. What if you had a function
> opal_paffinity_base_set_slots(long rank) (or some other name, I don't
> care) which looked up the mca parameter and then setup the slots as you
> are doing if it is fount. This would make things a bit cleaner IMHO.
>
> 2. the functions in the paffinety base should be prefixed with
> 'opal_paffinity_base_'
>
> 3. Why was the ompi_debug_flag added? It is not used anywhere.
>
> 4. You probably do not need to add the opal debug flag. There is already
> a 'paffinity_base_verbose' flag which should suit your purposes fine. So
> you should just be able to replace all of the conditional output
> statements in paffinity with something like
> opal_output_verbose(10, opal_paffinity_base_output, ...),
> where 10 is the verbosity level number.
>
> Tim
>
>
> Lenny Verkhovsky wrote:
>>
>>
>> Hi, all
>>
>> Attached patch for modified Rank_File RMAPS component.
>>
>>
>>
>> 1. introduced new general purpose debug flags
>>
>> mpi_debug
>>
>> opal_debug
>>
>>
>>
>> 2. introduced new mca parameter opal_paffinity_slot_list
>>
>> 3. ompi_mpi_init cleaned from opal paffinity functions
>>
>> 4. opal paffinity functions moved to new file
>> opal/mca/paffinity/base/paffinity_base_service.c
>>
>> 5. rank_file component files were renamed according to prefix policy
>>
>> 6. global variables renamed as well.
>>
>> 7. few bug fixes that were brought during previous discussions.
>>
>> 8. If user defines opal_paffinity_alone and rmaps_rank_file_path or
>> opal_paffinity_slot_list,
>>
>> then he gets a Warning that only opal_paffinity_alone will be used.
>>
>>
>>
>> .
>>
>> Best Regards,
>>
>> Lenny.
>>
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> devel mailing list
>> devel_at_[hidden]
>> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>
> _______________________________________________
> devel mailing list
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel