Skip to content

[torch/elastic] Improvements to the existing rendezvous types and a set of new unit tests for increased code coverage.#54807

Closed
cbalioglu wants to merge 4 commits intopytorch:masterfrom
cbalioglu:export-D27342444
Closed

[torch/elastic] Improvements to the existing rendezvous types and a set of new unit tests for increased code coverage.#54807
cbalioglu wants to merge 4 commits intopytorch:masterfrom
cbalioglu:export-D27342444

Conversation

@cbalioglu
Copy link
Contributor

@cbalioglu cbalioglu commented Mar 26, 2021

Summary: Improve the implementation and the unit test coverage of RendezvousParameters.

Differential Revision: D27342444

@facebook-github-bot facebook-github-bot added oncall: distributed Add this issue/PR to distributed oncall triage queue cla signed labels Mar 26, 2021
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 26, 2021 •
edited
Loading

💊 CI failures summary and remediations

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

  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See GitHub Actions build mypy (1/1)

Step: "Run mypy" (full log | diagnosis details | 🔁 rerun)

2021-03-27T01:29:39.8489183Z torch/distributed/elastic/rendezvous/api.py:262: error: "RendezvousParameters" has no attribute "backend" [attr-defined] 
2021-03-27T01:28:52.7674541Z shell: /bin/bash -e {0}
2021-03-27T01:28:52.7675073Z env:
2021-03-27T01:28:52.7675646Z   pythonLocation: /opt/hostedtoolcache/Python/3.8.8/x64
2021-03-27T01:28:52.7676414Z   LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.8.8/x64/lib
2021-03-27T01:28:52.7677106Z ##[endgroup]
2021-03-27T01:28:52.7768086Z + for CONFIG in mypy*.ini
2021-03-27T01:28:52.7769359Z + mypy --config=mypy-strict.ini
2021-03-27T01:29:07.6475775Z Success: no issues found in 37 source files
2021-03-27T01:29:08.3572602Z + for CONFIG in mypy*.ini
2021-03-27T01:29:08.3573763Z + mypy --config=mypy.ini
2021-03-27T01:29:39.8489183Z torch/distributed/elastic/rendezvous/api.py:262: error: "RendezvousParameters" has no attribute "backend"  [attr-defined]
2021-03-27T01:30:19.1435445Z Found 1 error in 1 file (checked 1276 source files)
2021-03-27T01:30:21.1695525Z ##[error]Process completed with exit code 1.
2021-03-27T01:30:21.1844723Z Post job cleanup.
2021-03-27T01:30:21.3003810Z [command]/usr/bin/git version
2021-03-27T01:30:21.3058305Z git version 2.31.0
2021-03-27T01:30:21.3104801Z [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand
2021-03-27T01:30:21.3147644Z [command]/usr/bin/git submodule foreach --recursive git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :
2021-03-27T01:30:21.3429688Z [command]/usr/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
2021-03-27T01:30:21.3461173Z http.https://github.com/.extraheader
2021-03-27T01:30:21.3471844Z [command]/usr/bin/git config --local --unset-all http.https://github.com/.extraheader
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 to the (internal) Dr. CI Users group.

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 26, 2021

This pull request was exported from Phabricator. Differential Revision: D27342444

@cbalioglu cbalioglu requested review from aivanou and kiukchung March 26, 2021 22:29
@cbalioglu cbalioglu closed this Mar 26, 2021
@cbalioglu cbalioglu reopened this Mar 26, 2021
@cbalioglu cbalioglu changed the title [torch/elastic] Improve the implementation of RendezvousParameters and add its unit tests. [torch/elastic] Improvements to the existing rendezvous types and a set of new unit tests for increased code coverage. Mar 26, 2021
@cbalioglu cbalioglu mentioned this pull request Mar 27, 2021
Closed
@cbalioglu @facebook-github-bot
[torch/elastic] Revise the rendezvous exception types.
cd030fa 
Differential Revision: D27327505

fbshipit-source-id: d13fa6c0b730c87608e1be8ba62bf3efcfd5a16a
@cbalioglu @facebook-github-bot
[torch/elastic] Expose a silent parameter in EtcdServer.
30a2328 
Differential Revision: D27327495

fbshipit-source-id: eed0e83018603050734615933505e94deab79c0d
@cbalioglu @facebook-github-bot
[torch/elastic] Improve the implementation of the utility functions a…
ddb6c88 
…nd add their unit tests.

Differential Revision: D27327898

fbshipit-source-id: fa389490b47f7a76ffe20ff92de4c25039eab781
@cbalioglu @facebook-github-bot
[torch/elastic] Improve the implementation of RendezvousParameters …
f9a89f3 
…and add its unit tests. (#146)

Summary:
Pull Request resolved: pytorch/elastic#146

Pull Request resolved: #54807

Improve the implementation and the unit test coverage of `RendezvousParameters`.

Test Plan: Run the existing and newly-introduced unit tests.

Differential Revision: D27342444

fbshipit-source-id: fdeee778d49cd82e97a0a11ef3cc3c0da9a92d04
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 27, 2021

This pull request was exported from Phabricator. Differential Revision: D27342444

@codecov
Copy link

codecov bot commented Mar 27, 2021

Codecov Report

Merging #54807 (f9a89f3) into master (394b720) will decrease coverage by 0.42%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #54807      +/-   ##
==========================================
- Coverage   77.44%   77.01%   -0.43%     
==========================================
  Files        1892     1892              
  Lines      186259   186272      +13     
==========================================
- Hits       144243   143453     -790     
- Misses      42016    42819     +803

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Apr 6, 2021

This pull request has been merged in 359d0a0.

@rgommers
Copy link
Collaborator

rgommers commented Apr 6, 2021

As reported on Slack, there's multiple issues with this PR and it broke mypy CI:

@t-vi: df299db broke the CI (mypy) because typing.final is Python 3.8+

@rgommers: This particular commit was landed together with #54807, however the commit itself is not part of that PR. Plus that PR landed as 4 commits instead of 1. On top of that, mypy CI for that PR was failing already on a different commit, and it was landed anyway. So it seems the regular review process wasn't followed here.

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

Reviewers

@H-Huang H-Huang Awaiting requested review from H-Huang

@mingzhe09088 mingzhe09088 Awaiting requested review from mingzhe09088

@mrshenli mrshenli Awaiting requested review from mrshenli

@pritamdamania87 pritamdamania87 Awaiting requested review from pritamdamania87

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

@wayi1 wayi1 Awaiting requested review from wayi1

@zhaojuanmao zhaojuanmao Awaiting requested review from zhaojuanmao

@kiukchung kiukchung Awaiting requested review from kiukchung

@aivanou aivanou Awaiting requested review from aivanou

Assignees

No one assigned

Labels

cla signed fb-exported Merged oncall: distributed Add this issue/PR to distributed oncall triage queue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cbalioglu @facebook-github-bot @rgommers