Repository navigation
Conversation
The mixed combine kept the shared staging condition, which moves domain-0 replies into the reserved staging slice only with two or more pure-Attention domains. With exactly one, a domain-0 Attention GPU's next dispatch writes its own tokens into its self-lane inbox over the default staging while its NIC may still be reading replies for the other domain. On 2 x 4 H200, BF16, capacity 512, the shipped tree mismatched in 4 of 4 runs; domain 0 now stages in the slice for any number of pure-Attention domains, at no memory or latency cost. Record in docs/validation-mixed-staging-fix.json. Signed-off-by: Itay Etelis <92247226+Etelis@users.noreply.github.com>
|
almogtavor
requested changes
Oct 27, 2026
| and a two-buffer interleave. | ||
| ([record](validation-mixed-hybrid.json)) | ||
|
|
||
| **Fix: domain-0 combine staging with one pure-Attention domain.** The mixed combine moved |
Member
There was a problem hiding this comment.
Don't talk in the docs about bugs and fixes, the user doesn't need to know there was a bug or a fix. Just talk about the current state
Member
Author
|
That's just an exploration I've done, I don't feel that's the right way to go, we need to completely loose the math-based approach DeepEP uses to map experts into ranks -- We rather have a lookup table, that way we don't have any assumptions on where / # of FFN worker. @almogtavor sorry for making you reviewing that. |
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
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 a race in the opt-in mixed-role path (
AFDEP_MIXED_HYBRID=1, design section 18) with exactly two NVLink domains.The bug: domain 0 stages its combine replies at the default offset when there is only one pure-Attention domain. A domain-0 Attention GPU's next dispatch then writes its own tokens there, while its NIC may still be reading those replies. The pure-Attention domain receives corrupted rows.
The fix: domain 0 always stages in the reserved slice. That's one condition in the generated mixed combine. The slice was already reserved, so memory and latency don't change.
Validation (kermit, 2–3 nodes × 4 H200)
mainmismatches in 4 of 4 GPU-suite runs at F = 1 and 2, BF16, capacity 512: 0.1–3.1% of the pure-Attention ranks' values.docs/validation-mixed-staging-fix.json.#22 carries the same mixed combine and will pick this up on merge.