Reject a jaccl hostfile that does not describe the links the backend uses - #4284
Open
erwinzhang7 wants to merge 1 commit into
Open
Reject a jaccl hostfile that does not describe the links the backend uses#4284erwinzhang7 wants to merge 1 commit into
erwinzhang7 wants to merge 1 commit into
Conversation
…uses launch_jaccl checked the shape of the rdma matrix, its dimensions and its null diagonal, but never whether the pairs the backend is about to communicate over are actually in it. A hostfile whose rank order places unconnected nodes beside each other passes both checks. What follows is the worst failure available. The group does not fail to form and it does not run slowly: every rank ends up alone in a group of size one, reports success, and the job computes N separate wrong answers at full speed with nothing on stderr. Verified on four M4 Pro minis wired as a full mesh, where permuting two hosts in a working hostfile, rows and columns together so the file stays internally consistent, produced four independent groups of one and four 'membership OK' lines. This is the same class as ml-explore#4210, where a group of size 1 counted as a successful init. missing_jaccl_links reports the pairs that are used but absent: for a ring each rank's two neighbours, for a mesh every pair. With two ranks the previous and next neighbour are the same node, which is harmless because the pair is simply examined twice. The error names the hosts and says that for jaccl-ring the host order is the ring order, since that is the mistake that produces it. Checked against the live mesh: the misordered hostfile is now rejected, and valid mesh and valid ring hostfiles still run at 20.87 and 16.33 GB/s.
zcbenz
added
the
await verification
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes #4283.
The existing checks cover the shape of the rdma matrix and its null diagonal, not
whether the pairs about to be used are present, so a hostfile whose rank order places
unconnected nodes beside each other passes and then leaves every rank alone in a
group of size one, reporting success. Same class as #4210.
missing_jaccl_linksreports the pairs that are used but absent: for a ring eachrank's two neighbours, for a mesh every pair. With two ranks the previous and next
neighbour are the same node, which is harmless because the pair is simply examined
twice. The error names the hosts and says that for jaccl-ring the host order is the
ring order, since that is the mistake that produces it.
Checked against a live four node mesh: the misordered hostfile is now rejected, a
ring hostfile used as a mesh is rejected, and valid mesh and valid ring hostfiles
still run at 20.87 and 16.33 GB/s respectively. Six unit tests cover mesh, ring, the two rank
edge case and a ring that is deliberately not a mesh.