Skip to content

Enable distributed package on windows, Gloo backend supported only#42897

Closed
gunandrose4u wants to merge 67 commits intopytorch:masterfrom
gunandrose4u:enable_win_dist
Closed

Enable distributed package on windows, Gloo backend supported only#42897
gunandrose4u wants to merge 67 commits intopytorch:masterfrom
gunandrose4u:enable_win_dist

Conversation

@gunandrose4u
Copy link
Copy Markdown
Contributor

@gunandrose4u gunandrose4u commented Aug 12, 2020

Fixes #42095

For test case part will be committed to this PR later

@mrshenli, please help to review

@gunandrose4u gunandrose4u mentioned this pull request Aug 12, 2020
Open
@dr-ci
Copy link
Copy Markdown

dr-ci bot commented Aug 12, 2020 •
edited
Loading

💊 CI failures summary and remediations

As of commit 8404d1e (more details on the Dr. CI page):

  • 2/2 failures possibly* introduced in this PR
    • 2/2 non-CircleCI failure(s)

1 failure confirmed as flaky and can be ignored:

  • pytorch_macos_10_13_py3_build

Extra GitHub checks: 1 failed

codecov.io: 1 failed

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 325 times.

@gunandrose4u gunandrose4u marked this pull request as draft August 12, 2020 04:14
mrshenli
mrshenli reviewed Aug 12, 2020
View reviewed changes
Comment thread CMakeLists.txt Outdated
Comment thread cmake/Dependencies.cmake Outdated
Comment thread torch/CMakeLists.txt Outdated
Comment thread torch/CMakeLists.txt Outdated
Copy link
Copy Markdown
Contributor

@mrshenli mrshenli Aug 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need to add NOT MSVC to the USE_NCCL flag as well. Looks like NCCL does not officially support Windows yet https://forums.developer.nvidia.com/t/is-there-a-nccl-2-x-for-windows/55659/3

Copy link
Copy Markdown
Contributor Author

@gunandrose4u gunandrose4u Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should add one.

Copy link
Copy Markdown
Contributor

@mrshenli mrshenli Sep 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious, I don't see a special handling for NCCL, but there is no test failures for NCCL backend. Is it because NCCL now works on Windows or we didn't actually trigger those tests?

mrshenli
mrshenli reviewed Aug 12, 2020
View reviewed changes
Comment thread torch/csrc/distributed/c10d/comm.h Outdated
Comment thread torch/csrc/distributed/c10d/init.cpp Outdated
Comment thread torch/csrc/distributed/c10d/reducer.cpp Outdated
Comment thread torch/csrc/distributed/rpc/rref_context.h Outdated
Comment thread torch/csrc/distributed/rpc/tensorpipe_agent.cpp Outdated
Comment thread torch/csrc/distributed/rpc/tensorpipe_agent.h Outdated
Comment thread torch/csrc/distributed/rpc/tensorpipe_agent.cpp Outdated
Comment thread torch/csrc/distributed/rpc/unpickled_python_call.h Outdated
mrshenli
mrshenli reviewed Aug 12, 2020
View reviewed changes
Comment thread torch/lib/c10d/FileStore.cpp Outdated
@zhangguanheng66 zhangguanheng66 added module: distributions Related to torch.distributions triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Aug 12, 2020
lw
lw reviewed Aug 14, 2020
View reviewed changes
Comment thread torch/csrc/distributed/c10d/comm.h Outdated
lw
lw reviewed Aug 14, 2020
View reviewed changes
Comment thread torch/csrc/distributed/rpc/init.cpp Outdated
lw
lw reviewed Aug 14, 2020
View reviewed changes
Comment thread torch/csrc/distributed/rpc/rref_context.h Outdated
lw
lw reviewed Aug 14, 2020
View reviewed changes
Comment thread torch/distributed/rpc/backend_registry.py Outdated
lw
lw reviewed Aug 14, 2020
View reviewed changes
Comment thread torch/distributed/rpc/constants.py Outdated
lw
lw reviewed Aug 14, 2020
View reviewed changes
Comment thread torch/lib/c10d/FileStore.cpp Outdated
lw
lw reviewed Aug 14, 2020
View reviewed changes
Comment thread torch/lib/c10d/GlooDeviceFactory.cpp Outdated
@gunandrose4u
Merge pull request #1 from pytorch/master
bf02d77 
Update my fork from Pytorch repo
@mrshenli mrshenli requested a review from malfet August 20, 2020 14:18
@mrshenli mrshenli added the module: build Build system issues label Aug 20, 2020
@gunandrose4u
Merge pull request #2 from pytorch/master
2ebe620 
Rebase from pytorch/pytorch master
zhuganghuaonnet and others added 24 commits September 25, 2020 15:16
@zhuganghuaonnet
trigger build
b5c4c95
@zhuganghuaonnet
Fix lint check failure
1e3e9fa
@zhuganghuaonnet
Trigger build
969b6ca
@zhuganghuaonnet
set libuv_ROOT env var
a0948c5
@gunandrose4u @zhuganghuaonnet
Resolve code review comments
67dd9cc
@gunandrose4u @zhuganghuaonnet
Remove TORCH_API and C10_EXPORT
058634e
@gunandrose4u @zhuganghuaonnet
Update
50febfc
@gunandrose4u @zhuganghuaonnet
Revert changes about TORCH_API
7792e64
@gunandrose4u @zhuganghuaonnet
Remove debug build script
eb1c50c
@gunandrose4u @zhuganghuaonnet
Try get active network interface in Windows test box
9c34fbd
@gunandrose4u @zhuganghuaonnet
Change test_c10d network interface name to test box one
3fd51b5
@gunandrose4u @zhuganghuaonnet
Resolve code review comments
f781770
@zhuganghuaonnet
Resolve code review comments
ee82569
@zhuganghuaonnet
Resolve code review comments
53174cb
@zhuganghuaonnet
Remove RPC related changes
4e7bea1
@zhuganghuaonnet
fix typo
31012cd
@zhuganghuaonnet
Resolve review comments
3fff975
@zhuganghuaonnet
Resolve all comments
96aa7e0
@zhuganghuaonnet
Try to fix non-Windows platform rpc test failure
88c3ec5
@zhuganghuaonnet
Revert "Try to fix non-Windows platform rpc test failure"
093894d 
This reverts commit defd3ac.
@zhuganghuaonnet
Resolve distributed/rpc/test_faulty_agent test failure on non Windows…
187b807 
… platform
@zhuganghuaonnet
Resolve code review comments
1677cdf
@malfet @zhuganghuaonnet
Update torch/lib/c10d/CMakeLists.txt
ba7d9f9
@zhuganghuaonnet
Fix Windows build error
8404d1e
@gunandrose4u gunandrose4u mentioned this pull request Sep 25, 2020
Closed
facebook-github-bot pushed a commit that referenced this pull request Sep 25, 2020
@gunandrose4u @facebook-github-bot
Fix Windows build failure after DDP PR merged (#45335)
f07ac6a 
Summary:
Fixes #{issue number}
This is resubmit for PR #42897 . Together with fix for Windows build issue introduced by PR #44344 .

Pull Request resolved: #45335

Reviewed By: zou3519

Differential Revision: D23931471

Pulled By: mrshenli

fbshipit-source-id: f49b5a114944c1450b32934b3292170be064f494
@gunandrose4u
Copy link
Copy Markdown
Contributor Author

gunandrose4u commented Sep 26, 2020

Merged again by PR f07ac6a

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

Reviewers

@lw lw lw left review comments

@mrshenli mrshenli mrshenli requested changes

@malfet malfet malfet approved these changes

@apaszke apaszke Awaiting requested review from apaszke

@beauby beauby Awaiting requested review from beauby

@jiayisuse jiayisuse Awaiting requested review from jiayisuse

@osalpekar osalpekar Awaiting requested review from osalpekar

@pietern pietern Awaiting requested review from pietern

@pritamdamania87 pritamdamania87 Awaiting requested review from pritamdamania87

@zhaojuanmao zhaojuanmao Awaiting requested review from zhaojuanmao

@rohan-varma rohan-varma Awaiting requested review from rohan-varma

+1 more reviewer

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Merged module: build Build system issues module: distributions Related to torch.distributions open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[RFC] Add Windows support to torch.distributed package