[C10D] support group_src/dst in broadcast/reduce ops #140843
Conversation
Also add mypy annotations [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/140843
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ✅ No FailuresAs of commit 8307988 with merge base b379a28 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
pytorch-bot
bot
added
oncall: distributed
Also add mypy annotations cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 d4l3k c-p-i-o [ghstack-poisoned]
Also add mypy annotations cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 d4l3k c-p-i-o [ghstack-poisoned]
Also add mypy annotations cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 d4l3k c-p-i-o [ghstack-poisoned]
Also add mypy annotations cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 d4l3k c-p-i-o [ghstack-poisoned]
Also add mypy annotations [ghstack-poisoned]
Also add mypy annotations [ghstack-poisoned]
Also add mypy annotations Partially addresses RFC 0042 (pytorch/rfcs#71) See more details/motivation in #140460 [ghstack-poisoned]
Also add mypy annotations Partially addresses RFC 0042 (pytorch/rfcs#71) See more details/motivation in #140460 [ghstack-poisoned]
| if group_rank: | ||
| c10d.broadcast(x, group_src=1, group=subgroup) | ||
| else: | ||
| c10d.broadcast(x, src=self.rank + 1, group=subgroup) |
There was a problem hiding this comment.
nit: not introduced by this PR but src=self.rank + 1 can be a potential cause for collective mismatch if someone scaling the world size for this test suit.
There was a problem hiding this comment.
is it? I saw a 'world_size=4' hardcode and a if self.rank >= world_size: return so i thought it was correct even though it insists on using exactly 4 gpus rather than being flexible to all available gpus
| global_src = _canonicalize_group_rank(group, src, group_src, return_global=True) | ||
| group_src = _canonicalize_group_rank(group, src, group_src, return_global=False) |
There was a problem hiding this comment.
Remind me why we need both?
There was a problem hiding this comment.
well, the existing code uses both actually. I prioritized changing less of the existing code in case of making mistakes. It might be possible to avoid having both.
There was a problem hiding this comment.
wait, you're right i think i can delete group_src here. Seems totally unused. Maybe i refactored more later and forgot to clean up.
Also add mypy annotations Partially addresses RFC 0042 (pytorch/rfcs#71) See more details/motivation in #140460 [ghstack-poisoned]
…0847) Also add mypy annotations Partially addresses RFC 0042 (pytorch/rfcs#71) See more details/motivation in #140460 Pull Request resolved: #140847 Approved by: https://github.com/H-Huang ghstack dependencies: #140843
Also add mypy annotations Partially addresses RFC 0042 (pytorch/rfcs#71) See more details/motivation in pytorch#140460 Pull Request resolved: pytorch#140843 Approved by: https://github.com/kwen2501
…orch#140847) Also add mypy annotations Partially addresses RFC 0042 (pytorch/rfcs#71) See more details/motivation in pytorch#140460 Pull Request resolved: pytorch#140847 Approved by: https://github.com/H-Huang ghstack dependencies: pytorch#140843
Also add mypy annotations ghstack-source-id: 1776a64 Pull Request resolved: pytorch/pytorch#140843
3 participants
Stack from ghstack (oldest at bottom):
Also add mypy annotations
Partially addresses RFC 0042 (pytorch/rfcs#71)
See more details/motivation in #140460
cc @H-Huang @awgu @kwen2501 @wanchaol @fegin @fduwjj @wz337 @d4l3k @c-p-i-o