-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Tools] use torchrun instead of torch.distributed.launch #6304
Conversation
fix typo in cleanup_proc
Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:
|
@Rhett-Ying I kindly ask to review this pull request. |
@9rum Thanks for filing this PR. have your verified in your local? |
please update test cases like dgl/tests/tools/test_launch.py Line 24 in ce8a7dd
|
Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:
|
@Rhett-Ying Thanks for your kind reply. ......
----------------------------------------------------------------------
Ran 6 tests in 0.000s
OK In addition to the above unit test, I have confirmed that the changes work well with the official distributed GraphSAGE training example. |
Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
This PR fixes future warning from
torch.distributed.launch
(discussed in issue #5493) with minor fix typo.The
torch.distributed.launch
module is deprecated and it always printsFutureWarning: The module torch.distributed.launch is deprecated and will be removed in future. Use torchrun.
.Note that the typo issue from
--local-rank
argument is already fixed.Changes
torchrun
instead oftorch.distributed.launch
: The aforementioned deprecation warning is eliminated.cleanup_proc
:cleanup_proc
no longer prints misspelled message.