Repository navigation
Conversation
Signed-off-by: Ting Sun <suntcrick@gmail.com>
Contributor
[Medium impact] The changes appear safe to merge; no new actionable issues were found. SummaryBounds retained scratch storage by reusing capacity buckets while keeping old buffers alive for CUDA graphs.
The earlier, unnumbered finding about unsupported machines is fixed: the test checks symmetric-memory support and peer access before starting workers. Reviews (3) · Last reviewed commit: "2x -> 25% memory overhead" · Reviewed by Greptile |
5 of 7 tasks
Signed-off-by: Ting Sun <suntcrick@gmail.com>
Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Member
|
/te-ci pytorch L0 |
Member
|
Part of the greptile AI comment also applies here: #2037 (comment) where the workload could be using more memory with no changes from user side. |
This branch has not been deployed
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.
Description
Symmetric all-reduce retains a staging allocation for every distinct input size. A long-lived dynamic inference engine can therefore accumulate buffers for past prefill lengths, even though it only needs one size at a time.
Round the internal scratch allocation to a power-of-two capacity and reduce only the requested slice. Retaining the capacity buckets keeps old CUDA graph addresses valid while bounding their total storage to less than twice the largest bucket, per dtype, device and process group. The allocation helper's exact-size behavior and the existing zero-copy path remain unchanged.
This is an alternative implementation of the memory reuse proposed in #2037. It retains old allocations so captured graphs can continue to use them after capacity grows.
Type of change
Changes
Reproduction and validation
Base:
eb3c860a4d3394b7eb5e21dba3021fa10782a52e.Candidate:
e860467c74fcdd5c6ed2324bee0ee346f4deb0ee.In a TE-supported PyTorch development environment with two peer-accessible GPUs, save the attached patch as
external-change.patch, then run:To run the same test against the original implementation in a disposable checkout, retain the new test and restore only the production file:
git show eb3c860a4d3394b7eb5e21dba3021fa10782a52e:transformer_engine/pytorch/distributed.py > transformer_engine/pytorch/distributed.py python -m pytest -q tests/pytorch/distributed/test_symmetric_memory.pyThe original implementation fails the retained-capacity assertion after 32 different row counts. The test also checks numerical reductions and old graph replay after another capacity is allocated.
The source-built candidate passed the focused test (
1 passed in 11.46s). The existing Linear sanity selection passed 486 cases; 954 unsupported parameter combinations were skipped by the upstream test. The Python L0 lint completed withYour code has been rated at 10.00/10; the license check passed.The candidate's common and PyTorch native extensions were built from source for H200 (SM90), using the repository-pinned NCCL 2.30.7, CUDA 13.1 and PyTorch 2.11.0a0+eb65b36914.nv26.02. Import checks confirmed that Python and both native libraries came from this checkout. Use an isolated environment without an older TE core distribution; the optional DeepEP extension from the original image was excluded because it was linked against another NCCL version and is not used by this dense model. NCCL NVLS was disabled because of the environment's multicast setup. The commands above target SM90 and require the project's build dependencies.
Supplemental model evidence uses Qwen3-32B through Megatron dynamic inference at TP4. The original cache retains 48.75 MiB per rank across 16 natural-text prompt lengths; a second sweep over those lengths adds no further buffers. The candidate retains 15 MiB. All four ranks match the original symmetric-reduction implementation exactly for generated tokens and log probabilities across solo, mixed and reversed requests. No OOM was observed, and this experiment does not establish production shape frequencies.
The full native build also passed Qwen3-32B BF16 TP4 eager inference: all four ranks matched solo/mixed/reversed tokens and log probabilities, with 6912 symmetric reductions per rank. The repeated length scan retained 15 MiB per rank and did not grow on the second sweep. Model-level decode CUDA graphs exercised 42 graph steps per rank; outputs matched each other and the eager run exactly. FlashAttention 4.0.0b35 was used. These are supplemental integration results; the full upstream CI suite has not been run.
Checklist: