[c10d] Error out the case when registering symmetric memory without eager init by fduwjj · Pull Request #160145 · pytorch/pytorch

fduwjj · 2025-08-07T22:19:52Z

Stack from ghstack (oldest at bottom):

-> [c10d] Error out the case when registering symmetric memory without eager init #160145

Instead of implicitly creating nccl comm inside mem pool registration for symmetric memory, we decide to error it out so that we only support eager init case when the nccl comm is already initiated.

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

…ager init [ghstack-poisoned]

pytorch-bot · 2025-08-07T22:19:54Z

🔗 Helpful Links

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

📄 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 e464581 with merge base f33ce40 ():
💚 Looks good so far! There are no failures yet. 💚

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

…ager init ghstack-source-id: ea4e1f4 Pull Request resolved: #160145

torch/csrc/distributed/c10d/ProcessGroupNCCL.cpp

…y without eager init" Instead of implicitly creating nccl comm inside mem pool registration for symmetric memory, we decide to error it out so that we only support eager init case when the nccl comm is already initiated. cc H-Huang awgu wanchaol fegin wz337 wconstab d4l3k pragupta [ghstack-poisoned]

…ager init ghstack-source-id: 26f2135 Pull Request resolved: #160145

test/distributed/test_c10d_nccl.py

…y without eager init" Instead of implicitly creating nccl comm inside mem pool registration for symmetric memory, we decide to error it out so that we only support eager init case when the nccl comm is already initiated. cc H-Huang awgu wanchaol fegin wz337 wconstab d4l3k pragupta [ghstack-poisoned]

…ager init ghstack-source-id: 23cefcf Pull Request resolved: #160145

torch/csrc/distributed/c10d/ProcessGroupNCCL.cpp

kwen2501

LGTM.
cc @syed-ahmed in case there are other thoughts. [Edit: just saw that @syed-ahmed has reviewed this PR too]

torch/csrc/distributed/c10d/ProcessGroupNCCL.cpp

…y without eager init" Instead of implicitly creating nccl comm inside mem pool registration for symmetric memory, we decide to error it out so that we only support eager init case when the nccl comm is already initiated. cc H-Huang awgu wanchaol fegin wz337 wconstab d4l3k pragupta [ghstack-poisoned]

…ager init ghstack-source-id: b03d79c Pull Request resolved: #160145

fduwjj · 2025-08-12T20:11:46Z

@pytorchbot rebase

pytorchmergebot · 2025-08-12T20:13:10Z

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

[ghstack-poisoned]

pytorchmergebot · 2025-08-12T20:13:22Z

Successfully rebased gh/fduwjj/179/orig onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/160145)

…ager init ghstack-source-id: 08bf205 Pull Request resolved: #160145

fduwjj · 2025-08-12T21:43:21Z

@pytorchbot merge

pytorchmergebot · 2025-08-12T21:50:00Z

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

ngimel · 2025-08-12T23:26:27Z

I'm slightly surprised you didn't need to change test_nccl_user_buffer_registration

pytorch/test/distributed/test_c10d_nccl.py

Lines 3130 to 3146 in e464581

    
           c10d.init_process_group( 
        
               backend="nccl", rank=self.rank, world_size=self.world_size, store=store 
        
           ) 
        
           device = torch.device(f"cuda:{self.rank}") 
        
           torch.cuda.set_device(self.rank) 
        
           pg = c10d.distributed_c10d._get_default_group() 
        
           backend = pg._get_backend(torch.device(device)) 
        
           # Use NCCL memory allocator 
        
           pool = torch.cuda.MemPool(backend.mem_allocator) 
        
           # allocate memory with ncclMemAlloc 
        
           with torch.cuda.use_mem_pool(pool): 
        
               tensor = torch.arange(1024 * 1024 * 2, device=device) 
        
           # register buffers to NCCL 
        
           backend.register_mem_pool(pool)

, is it because we are using nccl comms between tests and by the time we run it communicator is already created? Would this test fail if run standalone?

fduwjj · 2025-08-13T03:34:09Z

@ngimel Hmmm good point, is it because test_nccl_user_buffer_registration only belongs periodic? since it is using 4 GPUs?

fduwjj · 2025-08-13T03:54:06Z

@ngimel fixed in #160497

Fixed `test_nccl_user_buffer_registration ` due to #160145, somehow CI didn't capture it. Pull Request resolved: #160497 Approved by: https://github.com/ngimel

…ager init (#160145) Instead of implicitly creating nccl comm inside mem pool registration for symmetric memory, we decide to error it out so that we only support eager init case when the nccl comm is already initiated. Pull Request resolved: #160145 Approved by: https://github.com/kwen2501

Fixed `test_nccl_user_buffer_registration ` due to #160145, somehow CI didn't capture it. Pull Request resolved: #160497 Approved by: https://github.com/ngimel

…ager init (#160145) Instead of implicitly creating nccl comm inside mem pool registration for symmetric memory, we decide to error it out so that we only support eager init case when the nccl comm is already initiated. Pull Request resolved: #160145 Approved by: https://github.com/kwen2501

Fixed `test_nccl_user_buffer_registration ` due to #160145, somehow CI didn't capture it. Pull Request resolved: #160497 Approved by: https://github.com/ngimel

…ager init (pytorch#160145) Instead of implicitly creating nccl comm inside mem pool registration for symmetric memory, we decide to error it out so that we only support eager init case when the nccl comm is already initiated. Pull Request resolved: pytorch#160145 Approved by: https://github.com/kwen2501

Fixed `test_nccl_user_buffer_registration ` due to pytorch#160145, somehow CI didn't capture it. Pull Request resolved: pytorch#160497 Approved by: https://github.com/ngimel

…ager init (pytorch#160145) Instead of implicitly creating nccl comm inside mem pool registration for symmetric memory, we decide to error it out so that we only support eager init case when the nccl comm is already initiated. Pull Request resolved: pytorch#160145 Approved by: https://github.com/kwen2501

Fixed `test_nccl_user_buffer_registration ` due to pytorch#160145, somehow CI didn't capture it. Pull Request resolved: pytorch#160497 Approved by: https://github.com/ngimel

[c10d] Error out the case when registering symmetric memory without e…

e9d1e52

…ager init [ghstack-poisoned]

fduwjj added a commit that referenced this pull request Aug 7, 2025

[c10d] Error out the case when registering symmetric memory without e…

003fe59

…ager init ghstack-source-id: ea4e1f4 Pull Request resolved: #160145

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

fduwjj requested review from d4l3k, eqy, kwen2501, ngimel and syed-ahmed August 7, 2025 22:20

syed-ahmed reviewed Aug 7, 2025

View reviewed changes

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

fduwjj added a commit that referenced this pull request Aug 8, 2025

[c10d] Error out the case when registering symmetric memory without e…

3d62058

…ager init ghstack-source-id: 26f2135 Pull Request resolved: #160145

fduwjj requested a review from syed-ahmed August 8, 2025 04:21

Skylion007 reviewed Aug 8, 2025

View reviewed changes

test/distributed/test_c10d_nccl.py Outdated Show resolved Hide resolved

syed-ahmed mentioned this pull request Aug 8, 2025

Refactors symmetric memory creation through allocator interface #160202

Closed

fduwjj added a commit that referenced this pull request Aug 8, 2025

[c10d] Error out the case when registering symmetric memory without e…

5ef1b5d

…ager init ghstack-source-id: 23cefcf Pull Request resolved: #160145

fduwjj requested a review from Skylion007 August 8, 2025 21:14

wconstab reviewed Aug 11, 2025

View reviewed changes

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

kwen2501 approved these changes Aug 12, 2025

View reviewed changes

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

fduwjj added a commit that referenced this pull request Aug 12, 2025

[c10d] Error out the case when registering symmetric memory without e…

685f475

…ager init ghstack-source-id: b03d79c Pull Request resolved: #160145

fduwjj added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 12, 2025

Update

e464581

[ghstack-poisoned]

pytorchmergebot pushed a commit that referenced this pull request Aug 12, 2025

[c10d] Error out the case when registering symmetric memory without e…

2779388

…ager init ghstack-source-id: 08bf205 Pull Request resolved: #160145

pytorchmergebot added the merging label Aug 12, 2025

pytorchmergebot closed this in b1f4354 Aug 12, 2025

pytorchmergebot added Merged and removed merging labels Aug 12, 2025

fduwjj mentioned this pull request Aug 13, 2025

[c10d] Fix test test_nccl_user_buffer_registration #160497

Closed

github-actions bot deleted the gh/fduwjj/179/head branch September 13, 2025 02:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[c10d] Error out the case when registering symmetric memory without eager init#160145

[c10d] Error out the case when registering symmetric memory without eager init#160145
fduwjj wants to merge 5 commits intogh/fduwjj/179/basefrom
gh/fduwjj/179/head

fduwjj commented Aug 7, 2025 •

edited

Loading

Uh oh!

pytorch-bot bot commented Aug 7, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kwen2501 left a comment •

edited

Loading

Uh oh!

Uh oh!

fduwjj commented Aug 12, 2025

Uh oh!

pytorchmergebot commented Aug 12, 2025

Uh oh!

pytorchmergebot commented Aug 12, 2025

Uh oh!

fduwjj commented Aug 12, 2025

Uh oh!

pytorchmergebot commented Aug 12, 2025

Uh oh!

ngimel commented Aug 12, 2025

Uh oh!

fduwjj commented Aug 13, 2025

Uh oh!

fduwjj commented Aug 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

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

Uh oh!

Uh oh!

pytorch-bot bot commented Aug 7, 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/160145

✅ No Failures

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kwen2501 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fduwjj commented Aug 12, 2025

Uh oh!

pytorchmergebot commented Aug 12, 2025

Uh oh!

pytorchmergebot commented Aug 12, 2025

Uh oh!

fduwjj commented Aug 12, 2025

Uh oh!

pytorchmergebot commented Aug 12, 2025

Merge started

Uh oh!

ngimel commented Aug 12, 2025

Uh oh!

fduwjj commented Aug 13, 2025

Uh oh!

fduwjj commented Aug 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

fduwjj commented Aug 7, 2025 •

edited

Loading

pytorch-bot bot commented Aug 7, 2025 •

edited

Loading

kwen2501 left a comment •

edited

Loading