[c10d][fr] Record each individual collective being coalesced #151238

fduwjj · 2025-04-14T17:20:32Z

Stack from ghstack (oldest at bottom):

During the record of FR for coalesced collectives we are not consistent. For P2P ops, we log individual collectives into FR but for non-p2p ops, we don't do that. This PR is trying to make non-P2P also log individual collective into FR so that we can use script to check correctness of ops for each one of collectives coalesced.

Also the added unit test also address the unit test ask in the comment in #150863.

cc @H-Huang @awgu @wanchaol @fegin @wz337 @wconstab @d4l3k

[ghstack-poisoned]

pytorch-bot · 2025-04-14T17:20:36Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/151238

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 0accc80 with merge base f1f18c7 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

ghstack-source-id: c7e72a9 Pull Request resolved: #151238

During the record of FR for coalesced collectives we are not consistent. For P2P ops, we log individual collectives into FR but for non-p2p ops, we don't do that. This PR is trying to make non-P2P also log individual collective into FR so that we can use script to check correctness of ops for each one of collectives coalesced. Also the added unit test also address the unit test ask in the comment in #150863. cc H-Huang awgu wanchaol fegin wz337 wconstab d4l3k [ghstack-poisoned]

ghstack-source-id: d04a5c3 Pull Request resolved: #151238

d4l3k

LGTM

d4l3k · 2025-04-15T00:42:27Z

test/distributed/test_c10d_nccl.py

    def _verify_trace(self, t, include_collectives, timing_enabled, is_json):
        ver = t["version"]
-        self.assertEqual(ver, "2.5")
+        self.assertEqual(ver, "2.7")


why didn't this error when the version was incorrectly (?) 2.6 above?

oh it is a rebase slowness. The base should be 2.6...

d4l3k · 2025-04-15T00:45:09Z

torch/csrc/distributed/c10d/ProcessGroupNCCL.cpp

+        pgStatus_,
+        /*isP2P=*/false);
+    (void)trace_id;
+  }


We do this so we can detect when a coalesced event wasn't triggered or had invalid shape/dtype/etc?

right, that's exactly what we want to do.

torch/csrc/distributed/c10d/ProcessGroupNCCL.cpp

During the record of FR for coalesced collectives we are not consistent. For P2P ops, we log individual collectives into FR but for non-p2p ops, we don't do that. This PR is trying to make non-P2P also log individual collective into FR so that we can use script to check correctness of ops for each one of collectives coalesced. Also the added unit test also address the unit test ask in the comment in #150863. cc H-Huang awgu wanchaol fegin wz337 wconstab d4l3k [ghstack-poisoned]

fduwjj · 2025-04-15T17:24:32Z

@pytorchbot rebase

pytorchmergebot · 2025-04-15T17:26:04Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

pytorchmergebot · 2025-04-15T17:26:09Z

Rebase failed due to Command git -C /home/runner/work/pytorch/pytorch rebase refs/remotes/origin/viable/strict gh/fduwjj/126/orig returned non-zero exit code 1

warning: skipped previously applied commit 03dc140ff4d
hint: use --reapply-cherry-picks to include skipped commits
hint: Disable this message with "git config set advice.skippedCherryPicks false"
Rebasing (1/2)
Rebasing (2/2)
Auto-merging test/distributed/test_c10d_nccl.py
CONFLICT (content): Merge conflict in test/distributed/test_c10d_nccl.py
Auto-merging torch/csrc/distributed/c10d/FlightRecorder.hpp
CONFLICT (content): Merge conflict in torch/csrc/distributed/c10d/FlightRecorder.hpp
Auto-merging torch/csrc/distributed/c10d/ProcessGroupNCCL.cpp
error: could not apply cbd06005993... [c10d][fr] Record each individual collective being coalesced
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Could not apply cbd06005993... [c10d][fr] Record each individual collective being coalesced

Raised by https://github.com/pytorch/pytorch/actions/runs/14475581233

During the record of FR for coalesced collectives we are not consistent. For P2P ops, we log individual collectives into FR but for non-p2p ops, we don't do that. This PR is trying to make non-P2P also log individual collective into FR so that we can use script to check correctness of ops for each one of collectives coalesced. Also the added unit test also address the unit test ask in the comment in #150863. cc H-Huang awgu wanchaol fegin wz337 wconstab d4l3k [ghstack-poisoned]

ghstack-source-id: 1d254e8 Pull Request resolved: #151238

fduwjj · 2025-04-15T17:34:39Z

@pytorchbot merge

pytorchmergebot · 2025-04-15T17:36:25Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

…#151238) During the record of FR for coalesced collectives we are not consistent. For P2P ops, we log individual collectives into FR but for non-p2p ops, we don't do that. This PR is trying to make non-P2P also log individual collective into FR so that we can use script to check correctness of ops for each one of collectives coalesced. Also the added unit test also address the unit test ask in the comment in pytorch#150863. Pull Request resolved: pytorch#151238 Approved by: https://github.com/d4l3k, https://github.com/wconstab ghstack dependencies: pytorch#151247

[c10d][fr] Record each individual collective being coalesced

aa978f5

[ghstack-poisoned]

pytorch-bot bot added oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: distributed (c10d) release notes category labels Apr 14, 2025

fduwjj added a commit that referenced this pull request Apr 14, 2025

[c10d][fr] Record each individual collective being coalesced

a356011

ghstack-source-id: c7e72a9 Pull Request resolved: #151238

fduwjj requested review from kwen2501, eqy and d4l3k April 14, 2025 18:10

fduwjj added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 14, 2025

This was referenced Apr 14, 2025

[c10d][fr] Enable FR analysis script for all fast-path coalesce op #151243

Closed

[c10d][fr] Enable FR analysis script for rest of all coalesce op #151247

Closed

fduwjj requested a review from wconstab April 14, 2025 18:10

fduwjj added a commit that referenced this pull request Apr 14, 2025

[c10d][fr] Record each individual collective being coalesced

827059d

ghstack-source-id: d04a5c3 Pull Request resolved: #151238

fduwjj closed this Apr 14, 2025

fduwjj reopened this Apr 14, 2025

d4l3k approved these changes Apr 15, 2025

View reviewed changes

wconstab approved these changes Apr 15, 2025

View reviewed changes

torch/csrc/distributed/c10d/ProcessGroupNCCL.cpp Outdated Show resolved Hide resolved

fduwjj added a commit that referenced this pull request Apr 15, 2025

[c10d][fr] Record each individual collective being coalesced

2f7cbf5

ghstack-source-id: 1d254e8 Pull Request resolved: #151238

pytorchmergebot added the merging label Apr 15, 2025

pytorchmergebot added the Merged label Apr 15, 2025

pytorchmergebot closed this in 71e7dcd Apr 15, 2025

pytorchmergebot removed the merging label Apr 15, 2025

github-actions bot deleted the gh/fduwjj/126/head branch May 25, 2025 02:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[c10d][fr] Record each individual collective being coalesced #151238

[c10d][fr] Record each individual collective being coalesced #151238

Uh oh!

fduwjj commented Apr 14, 2025 •

edited

Loading

Uh oh!

pytorch-bot bot commented Apr 14, 2025 •

edited

Loading

Uh oh!

d4l3k left a comment

Uh oh!

d4l3k Apr 15, 2025

Uh oh!

fduwjj Apr 15, 2025

Uh oh!

d4l3k Apr 15, 2025

Uh oh!

fduwjj Apr 15, 2025

Uh oh!

Uh oh!

fduwjj commented Apr 15, 2025

Uh oh!

pytorchmergebot commented Apr 15, 2025

Uh oh!

pytorchmergebot commented Apr 15, 2025

Uh oh!

fduwjj commented Apr 15, 2025

Uh oh!

pytorchmergebot commented Apr 15, 2025

Uh oh!

Uh oh!

[c10d][fr] Record each individual collective being coalesced #151238

[c10d][fr] Record each individual collective being coalesced #151238

Uh oh!

Conversation

fduwjj commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Apr 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/151238

✅ No Failures

Uh oh!

d4l3k left a comment

Choose a reason for hiding this comment

Uh oh!

d4l3k Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

fduwjj Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

d4l3k Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

fduwjj Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fduwjj commented Apr 15, 2025

Uh oh!

pytorchmergebot commented Apr 15, 2025

Uh oh!

pytorchmergebot commented Apr 15, 2025

Uh oh!

fduwjj commented Apr 15, 2025

Uh oh!

pytorchmergebot commented Apr 15, 2025

Merge started

Uh oh!

Uh oh!

fduwjj commented Apr 14, 2025 •

edited

Loading

pytorch-bot bot commented Apr 14, 2025 •

edited

Loading