Rework the MPI connect/accept code #12586
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add more info when something bad happens during the handshake. Signed-off-by: George Bosilca <[email protected]>
I still don't think this code is correct, but somehow it works. My main concern is that the PML add_proc will be called with a different list of processes, or at least in a different order, everywhere. Before this patch it worked because we were calling it with ALWAYS 0 procs (because by the time we got out of the loop the ilist was empty), so it was not doing much. But, why was it working without a proper add_proc ? Do we still need that add_proc there ? Signed-off-by: George Bosilca <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm puzzled by this code working. It does all the right things, but it was calling PML->add_procs with zero new procs, ALWAYS (the
ilistwas empty by the time we got to the add_procs call). But somehow it still worked, which seems to indicate that some of the PML are more tolerant to not having procs added correctly. But I'm sure that's not the case for all PML, especially for those requiring symmetric add_procs.Thus, at this point this PR is a bandaid, trying to do the right thing, at least partially. I think the correct solution is to remove the
ilistand always use themlist(the list of all, local and remote, processes) to do the add_procs, even if we are registering again already known processes. This is the only way to have a symmetric add_proc everywhere.