Fix pyi annotation for ProcessGroupNCCL.Options #130957
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/130957
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (2 Unrelated Failures)As of commit e9adf21 with merge base 1eedb0a ( BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: 1 jobs have failed, first few of them are: Check mergeability of ghstack PR / ghstack-mergeability-check Details for Dev Infra teamRaised by workflow job |
|
Yeah, We need to make this pyi more accurate and update-to-date.. |
torch/_C/_distributed_c10d.pyi
Outdated
| @property | ||
| def _is_high_priority_stream(self) -> bool: ... |
There was a problem hiding this comment.
Did you rebase before the change?
There was a problem hiding this comment.
No, looks like I do need to rebase
torch/_C/_distributed_c10d.pyi
Outdated
|
|
||
| class Options: |
There was a problem hiding this comment.
did we delete the _timeout intentionally from NCCLOptions?
There was a problem hiding this comment.
I checked the bindings and did not see timeout
|
When I rebased, it looked like someone @shuqiangzhang had modified these annotations as part of adding the split_group API. However, I double checked the annotations and I like mine better (as they are more accurate, and they don't use the @Property decorator unnecessarily). |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: 1 mandatory check(s) failed. The first few are: Dig deeper by viewing the failures on hud |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Stack from ghstack (oldest at bottom):
Probably all the other options need updating too, but this is the one I
needed. The accurate annotation was determined by reading
torch/csrc/distributed/c10d/init.cpp
Signed-off-by: Edward Z. Yang ezyang@meta.com