RFC: ompi_mpi_params.c: set mpi_add_procs_cutoff default to 0

[jsquyres]

Decrease the default value of the "mpi_add_procs_cutoff" MCA param from 1024 to 32.

@hppritcha Setting the milestone to v2.0.0 because the "partial add procs" behavior was supposed to be a key feature of v2.0.0. Setting that to only happen at 1024 procs seems a little high. Do you agree?

RFC for the rest of the community: does anyone have a concern about setting this value to 32?

[lanl-ompi]

Test FAILed.

[lanl-ompi]

Test FAILed.

[hppritcha]

I agree. Setting to 1024 by default is a recipe for not getting tested often.

Von meinem iPhone gesendet

Am 03.02.2016 um 19:50 schrieb Jeff Squyres notifications@github.com:

Decrease the default value of the "mpi_add_procs_cutoff" MCA param from 1024 to 32.

@hppritcha Setting the milestone to v2.0.0 because the "partial add procs" behavior was supposed to be a key feature of v2.0.0. Setting that to only happen at 1024 procs seems a little high. Do you agree?

RFC for the rest of the community: does anyone have a concern about setting this value to 32?

You can view, comment on, or merge this pull request online at:

#1340

Commit Summary

ompi_mpi_params.c: set mpi_add_procs_cutoff default to 32
File Changes

M ompi/runtime/ompi_mpi_params.c (9)
Patch Links:

https://github.com/open-mpi/ompi/pull/1340.patch
https://github.com/open-mpi/ompi/pull/1340.diff
—
Reply to this email directly or view it on GitHub.

[lanl-ompi]

Test FAILed.

[ggouaillardet]

@jsquyres you missed a , at the end of this line

[rhc54]

@jsquyres Only quibble I have is stating that it upping the value might impact communication performance. I don't think that is true - it might impact the latency of the first message, but it will have no impact on performance.

[jsquyres]

@hjelmn Per our IM discussion yesterday, can you share your performance testing results of setting mca_add_procs_cutoff to low values (including 0)?

[ggouaillardet]

btw, what matters more to get the best performance

• the number of MPI tasks ?
• the number of tasks per node ?
  (iirc, there are some optimizations to handle several tasks per node, so I guess the number of nodes is more relevant than the number of MPI tasks alone)

[rhc54]

Launch performance is more impacted when used in conjunction with the direct modex param. Memory footprint is the primary target for this param when used by itself.

So if we want to show a major improvement in launch performance, we'd need to turn both params "on". Not sure if the direct modex one has actually been tested enough for that step. Maybe we should turn it "on" by default in master for awhile?

[ggouaillardet]

can you remind me what direct modex is ?

• is this mpirun-less start (e.g. srun a.out) ?
• is this async_modex ? (and if no, should we turn this on by default too ?)

[rhc54]

Sorry - yes, it is the async_modex parameter (which results in the PMIx
direct modex operation being used). Basically, it means the current modex
blocking barrier is replaced with a "no-op" and data is only fetched as
required.

On Thu, Feb 4, 2016 at 3:59 AM, Gilles Gouaillardet <
notifications@github.com> wrote:

can you remind me what direct modex is ?

• is this mpirun-less start (e.g. srun a.out) ?
• is this async_modex ? (and if no, should we turn this on by default
  too ?)

—
Reply to this email directly or view it on GitHub
#1340 (comment).

[jsquyres]

@hjelmn I'm not sure why you say that this extra assignment is necessary (in jsquyres@cd5bad8#commitcomment-15887486). Even if we come through registration a 2nd time, we don't want to reset the default if the user already set a different value. What am I missing here?

Regardless, we should either be doing this extra assignment for all MCA params, or no MCA params -- if there's a reason to do the extra assignment for this MCA param, that same reason should apply to all MCA params, right?

[hjelmn]

I think we should set the variable back to the default. That is the way I have done it pretty much everywhere throughout the code base. See the other registrations in this same function.

If we come through registration twice it means we finalized the project or component. I don't think we should use a possibly stale value.

[hjelmn]

I should note this is the default behavior of dynamically loaded components. If the component is unloaded all its variables will automatically be reset on the next dlopen. I was trying to be consistent.

[jsquyres]

@hjelmn and I chatted on the phone -- he filled me in on what I was missing: there's a possible inconsistency in behavior with a program that does something like this:

MPI_T_Init(...);
MPI_T_Cvar_read(..., &value_pre);
MPI_T_Cvar_write(...);
MPI_T_Finalize(...);

MPI_Init(...)
MPI_T_Init(...);
MPI_T_Cvar_read(..., &value_post);
assert(value_pre == value_post);

When the cvar is part of a dynamically-loaded component, the assert will be true. If we remove the additional assignment, when the cvar is part of a statically-loaded component, the assert will be false. Nathan's second assignment ensures that the assert will be true in both cases.

This is actually a larger question for the MPI Tools Working Group in the MPI Forum: what consistency guarantees -- if any -- are provided by the MPI_T API when MPI_T and/or MPI is finalized?

For this PR, I'll remove my deletion of the 2nd assignment, and we'll leave OMPI's behavior in this area as it was. Nathan and I will bring up this what-does-MPI_T-guarantee issue with the Tools WG separately.

[jsquyres]

Per Feb 9 2016 webex, we decided to set the value to 0 on the rationale that dynamic add procs stuff is not used for processes that are on the same server -- so therefore 0 is not a whole lot different than 32. So this PR is now updated to set the default add procs cutoff to 0.