fix(pcie): preserve default all-reduce output lifetime#80
Open
yatesdr wants to merge 1 commit into
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Summary
Preserve normal out-of-place lifetime semantics for
PCIeDmaAllReduce.all_reduce()whenoutis omitted.The persistent default-output workspace added by #76 returns the same storage
for every call. A transformer can retain one all-reduce result across the next
collective: the embedding result becomes the layer-0 residual, then the
attention output projection performs another all-reduce. Reusing one output
buffer silently overwrites that live residual.
This PR:
torch.empty_like(inp)for the default-output form;out=path;storage, and revalidate earlier contents after later collectives;
This is intentionally based on
fix/pcie-dma-persistent-output-20260723, the head branch of #76, becauseSparkInfer
masterdoes not currently contain the affected implementation.Integration bug report and full model-level evidence:
local-inference-lab/vllm#181
Reproduction and evidence
Standalone four-rank production-geometry proof:
https://gist.github.com/yatesdr/a2e84aa3171ee0b355649704f04f96a8
Geometry:
2735 x 6144BF16, 33,607,680 bytes/result, four ranks,i8_ring.Before:
After:
The collective output hashes are unchanged.
In an end-to-end TP4/DCP4/MTP3 GLM-5.2 trace, the affected layer-0 residual
was bit-identical to the attention output and differed from the layer input in
6,141/6,144 BF16 values. With this fix it is bit-identical to the layer input
on all four ranks.
Validation
python3 -m py_compileon both modified Python filesgit diff --checkScope
This fixes the confirmed storage-lifetime corruption. It is not presented as
the complete fix for the separately tracked GLM-5.2 long-context retrieval
regression: the frozen 350k request still missed after this corruption was
removed.