coll/tuned: two_proc algo errors when forced on comm size > 2

[ggouaillardet]

No description provided.

[ggouaillardet]

@bosilca can you please review this ?
the discussion initially started at http://www.open-mpi.org/community/lists/users/2016/05/29081.php

[bosilca]

@ggouaillardet I think the knowledge about the number of processes should remain in the function itself (instead of being delegated to the decision_fixed file). What do you think?

[ggouaillardet]

the pro of my approach is there is no performance impact (a few nanoseconds that do not really matter though) when two_proc algorithm is not forced (e.g. chosen by default)
that is not a strong opinion, and I am fine moving the check if you believe that makes the code more maintainable.

[bosilca]

It is the same test just down in the function because down there we already have the comm size and then the test is a single branch.

[ggouaillardet]

by "down" you meant the test should be moved into mca_coll_base_*_two_proc right ?
in the case of barrier, the base function currently assumes the comm size is two.

[bosilca]

yes, in the *_two_proc functions. If the test is down there anybody can use them without prior knowledge of their requirements. In addition, if we return a specialized error code (MPI_NOT_SUPPORTED) the upper layer can them fallback to calling another function instead of simply returning the error.

[ggouaillardet]

ok, so from an end user point of view, forcing the two_proc algorithm on a 3 tasks communicator will ultimately result into invoking an other (default ?) algorithm.
did I get it right ?

[bosilca]

I just said it will be possible ;) I didn't suggested to do that in tuned.

[jsquyres]

@bosilca So is that a thumbs up or no?

[bosilca]

👍

[jsquyres]

I'm ok deferring this to v2.0.1. Can someone make a PR for v2.0.1?

[ggouaillardet]

i made the PR open-mpi/ompi-release#1154