-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Fix batch_isend_irecv tests for err case #63112
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
Conversation
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 87ee5ac (more details on the Dr. CI page):
🕵️ 1 new failure recognized by patternsThe following CI failures do not appear to be due to upstream breakages:
|
| reqs = dist.batch_isend_irecv([send_op]) | ||
| for req in reqs: | ||
| req.wait() |
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.
I'm assuming this unit test passed since the code after dist.batch_isend_irecv was never executed and batch_isend_irecv just threw an exception. If so, should we just call dist.batch_isend_irecv here and ignore the return value?
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.
Yes, I agree, will update soon.
|
@pritamdamania87 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@pritamdamania87 I have resolved your review comment. |
Codecov Report
@@ Coverage Diff @@
## master #63112 +/- ##
==========================================
- Coverage 66.47% 66.39% -0.08%
==========================================
Files 725 725
Lines 93457 93448 -9
==========================================
- Hits 62122 62046 -76
- Misses 31335 31402 +67 |
|
@pritamdamania87 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: - `batch_isend_irecv` returns a list of requests instead of a single request - remove some unused variables Pull Request resolved: #63112 Reviewed By: pbelevich, wayi1, fduwjj Differential Revision: D30921265 fbshipit-source-id: e2075925172805d33974ef0de6fb631bdf33b5ea
batch_isend_irecvreturns a list of requests instead of a single request