Skip to content

Test mFSDP v2 overlap with default and symmetric memory - #5859

Merged
wujingyue merged 16 commits into
NVIDIA:mainfrom
wujingyue:agent/zero-cta-overlap-test
Jul 21, 2026
Merged

Test mFSDP v2 overlap with default and symmetric memory#5859
wujingyue merged 16 commits into
NVIDIA:mainfrom
wujingyue:agent/zero-cta-overlap-test

Conversation

@wujingyue

@wujingyue wujingyue commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Parameterize test_overlaps_communication_and_compute so it runs both the existing default collective path and the symmetric-memory path.
  • Use a dedicated NCCL subgroup for the test mesh; the symmetric-memory case applies NCCL's zero-CTA policy while the default case uses a normal NCCL subgroup.
  • Initialize the default process group lazily in the test without fixture-level device_id eager init, and explicitly warm the zero-CTA subgroup before constructing the DeviceMesh.
  • Attribute profiler device kernels through their linked CPU parent operation using collect_linked_kernels, so all-gather, reduce-scatter, and GEMM kernels are found without depending on device-side kernel names.
  • Filter linked profiler events to CUDA kernels by activity_type, so zero-CTA all-gather copy-engine Memcpy events are intentionally excluded.
  • In the symmetric-memory zero-CTA case, assert that all-gather emits no all-gather kernels while reduce-scatter still emits kernels and overlaps compute.
  • Keep the same overlap expectations for default all-gather and for reduce-scatter in both modes on the existing large dim=16384 workload.

Why

The overlap test should cover symmetric-memory mFSDP behavior in addition to the default path. The symmetric-memory zero-CTA all-gather is copy-engine work rather than an SM-launched NCCL kernel, and GEMM kernel names vary across GPU architectures, so profiler attribution by linked CPU parent operation is more robust than matching device-side kernel names directly.

Impact

Test-only change. There is no production-code behavior change.

Related to #5655.

Validation

  • uv run --no-sync isort tests/unit_tests/distributed/mfsdp_v2/profiler_utils.py tests/unit_tests/distributed/mfsdp_v2/test_fully_shard.py
  • uv run --no-sync python -m compileall tests/unit_tests/distributed/mfsdp_v2/profiler_utils.py tests/unit_tests/distributed/mfsdp_v2/test_fully_shard.py
  • git diff --check
  • uv run --no-sync python -m pytest --collect-only -q tests/unit_tests/distributed/mfsdp_v2/test_fully_shard.py::test_overlaps_communication_and_compute

Full distributed CUDA execution is left to CI.

@copy-pr-bot

copy-pr-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@wujingyue
wujingyue force-pushed the agent/zero-cta-overlap-test branch 2 times, most recently from 31641d3 to 87b36e0 Compare July 18, 2026 05:02
@wujingyue wujingyue changed the title Tighten mFSDP v2 overlap testing with zero-CTA all-gather Tighten mFSDP v2 overlap test across default and symmetric memory Jul 18, 2026
@wujingyue wujingyue changed the title Tighten mFSDP v2 overlap test across default and symmetric memory Test mFSDP v2 overlap with default and symmetric memory Jul 18, 2026
@wujingyue
wujingyue marked this pull request as ready for review July 18, 2026 06:14
@wujingyue

Copy link
Copy Markdown
Contributor Author

/ok to test f0bcac2

@wujingyue

Copy link
Copy Markdown
Contributor Author

/ok to test 8470c815a54f1951b405c4b6bca8056e807b7a7d

@wujingyue

Copy link
Copy Markdown
Contributor Author

/ok to test 8470c81

wujingyue and others added 13 commits July 21, 2026 04:37
Use DeviceType.CUDA and DeviceType.CPU instead of stringly-typed device_type.name comparisons in the mfsdp_v2 profiler helper.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
Signed-off-by: Jingyue Wu <jingyuew@nvidia.com>
@wujingyue
wujingyue force-pushed the agent/zero-cta-overlap-test branch from 949a9df to 85178ba Compare July 21, 2026 04:40
@copy-pr-bot

copy-pr-bot Bot commented Jul 21, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@wujingyue
wujingyue enabled auto-merge July 21, 2026 04:41
@wujingyue

Copy link
Copy Markdown
Contributor Author

/claude fix

Signed-off-by: svcnvidia-nemo-ci <svcnvidia-nemo-ci@nvidia.com>
@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🛠️ Claude fix commit f9876cf2a5e1 (attempt 1)

⚠️ This explanation is AI-generated and may be inaccurate; the exact commit is authoritative.

What changed
No changes made. The base merge in pr-head/ was already fully resolved: no conflict markers in any of the four PR-touched files (transformer_engine.py, packed_seq_params.py, training.py, test_cuda_graphs.py), and all four parse as valid Python. Verified the training.py merge is correct — the app_train_loop_finish_time/app_finish_time logging in pretrain (normal path) is distinct from the early-exit/cleanup logging added in train(), so it is not a merge-induced duplicate.

Files changed by Claude

  • No additional file edits; the pinned base was merged.

Why
No conflict or failure to fix. Git reports all conflicts resolved with no markers present; the CI-log dir (/home/runner/work/_temp/claude-fix-ci/) and steering file are both empty, so there are no terminal lint or non-GB200 unit failures to act on. Lint tools (isort/black/ruff) are not installed, so those checks are unsupported and left unchanged per policy.

DCO
@wujingyue, please fix any DCO failures on your commits before merge. DCO does not block this workflow.

View exact commit

Sanitized and posted by svcnvidia-nemo-ci.

@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

/ok to test f9876cf

@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/29837017618

@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

✅ Claude fix CI passed after attempt 1. View exact-SHA CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Symmetric memory support in MFSDP v2

3 participants