{"action":"created","issue":{"url":"https://api.github.com/repos/pytorch/pytorch/issues/14142","repository_url":"https://api.github.com/repos/pytorch/pytorch","labels_url":"https://api.github.com/repos/pytorch/pytorch/issues/14142/labels{/name}","comments_url":"https://api.github.com/repos/pytorch/pytorch/issues/14142/comments","events_url":"https://api.github.com/repos/pytorch/pytorch/issues/14142/events","html_url":"https://github.com/pytorch/pytorch/pull/14142","id":381823298,"node_id":"MDExOlB1bGxSZXF1ZXN0MjMxNzAzNjQz","number":14142,"title":"[c10d] Make NCCL backend support barrier op","user":{"login":"teng-li","id":8120856,"node_id":"MDQ6VXNlcjgxMjA4NTY=","avatar_url":"https://avatars2.githubusercontent.com/u/8120856?v=4","gravatar_id":"","url":"https://api.github.com/users/teng-li","html_url":"https://github.com/teng-li","followers_url":"https://api.github.com/users/teng-li/followers","following_url":"https://api.github.com/users/teng-li/following{/other_user}","gists_url":"https://api.github.com/users/teng-li/gists{/gist_id}","starred_url":"https://api.github.com/users/teng-li/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/teng-li/subscriptions","organizations_url":"https://api.github.com/users/teng-li/orgs","repos_url":"https://api.github.com/users/teng-li/repos","events_url":"https://api.github.com/users/teng-li/events{/privacy}","received_events_url":"https://api.github.com/users/teng-li/received_events","type":"User","site_admin":false},"labels":[{"id":679953883,"node_id":"MDU6TGFiZWw2Nzk5NTM4ODM=","url":"https://api.github.com/repos/pytorch/pytorch/labels/distributed","name":"distributed","color":"c2e0c6","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-11-17T03:29:48Z","updated_at":"2018-11-21T04:19:09Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/pytorch/pytorch/pulls/14142","html_url":"https://github.com/pytorch/pytorch/pull/14142","diff_url":"https://github.com/pytorch/pytorch/pull/14142.diff","patch_url":"https://github.com/pytorch/pytorch/pull/14142.patch"},"body":"This is a feature request from: https://github.com/pytorch/pytorch/issues/13573\r\n\r\nAs the title says, this PR makes NCCL backend support barrier op.\r\n\r\nThere are a couple scenarios that need to be addressed:\r\n(1) When there is already a NCCL op happened, we need to record what GPU device(s)  the previous op happened and queue the allreduce barrier op on the same GPU device\r\n(2) When there is no NCCL op yet, we will try to use a single GPU and separate each process from a single GPU as the best effort.\r\n\r\nAs for the async work, during wait, we would like not just wait on the NCCL kernel to be completed, but also block the thread until the current stream and nccl stream return.\r\n\r\n`test_distributed` should cover the test. I also manually tested both scenarios. "},"comment":{"url":"https://api.github.com/repos/pytorch/pytorch/issues/comments/440524384","html_url":"https://github.com/pytorch/pytorch/pull/14142#issuecomment-440524384","issue_url":"https://api.github.com/repos/pytorch/pytorch/issues/14142","id":440524384,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0MDUyNDM4NA==","user":{"login":"pietern","id":9845,"node_id":"MDQ6VXNlcjk4NDU=","avatar_url":"https://avatars3.githubusercontent.com/u/9845?v=4","gravatar_id":"","url":"https://api.github.com/users/pietern","html_url":"https://github.com/pietern","followers_url":"https://api.github.com/users/pietern/followers","following_url":"https://api.github.com/users/pietern/following{/other_user}","gists_url":"https://api.github.com/users/pietern/gists{/gist_id}","starred_url":"https://api.github.com/users/pietern/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pietern/subscriptions","organizations_url":"https://api.github.com/users/pietern/orgs","repos_url":"https://api.github.com/users/pietern/repos","events_url":"https://api.github.com/users/pietern/events{/privacy}","received_events_url":"https://api.github.com/users/pietern/received_events","type":"User","site_admin":false},"created_at":"2018-11-21T04:19:09Z","updated_at":"2018-11-21T04:19:09Z","author_association":"CONTRIBUTOR","body":"@teng-li It is only the same for MPI because everything is serialized. There is simply no alternative.\r\n\r\nI'm fine with these semantics, and I agree they are useful (less code to wait for everything to complete). BUT, then it should also be documented as such, and tested as such. In a test you can kick off some async work, run a barrier, and check that all tensors have the expected results.\r\n\r\nI also think there is an alternative approach to this, given the semantic you're looking for, that is more robust. The last devices list assumes that you're using a single NCCL communicator, and that you ensure everything is completed by queuing something against that same communicator. This is only true if you have a single communicator, but breaks down when you have multiple. To fix this, you can keep a map with a single CUDA event per NCCL communicator, that you record immediately after queuing some operation on that communicator. Then when you execute a barrier, you can use a single device communicator, grab a stream, wait for the CUDA events of all other communicators, and then run the allreduce. With this approach nothing can fall through the cracks when using multiple communicators.\r\n\r\nThis semantic should also be implemented for the Gloo backend."}}
{"actor":{"display_login":"pietern"}}