-
Notifications
You must be signed in to change notification settings - Fork 409
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
Delete Device2Host caused by comm with device and host #2840
base: master
Are you sure you want to change the base?
Delete Device2Host caused by comm with device and host #2840
Conversation
for more information, see https://pre-commit.ci
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.
can we pls add tests for this fix?
and maybe pls update the title to better express what the PR does
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2840 +/- ##
========================================
- Coverage 69% 41% -28%
========================================
Files 346 332 -14
Lines 19129 18961 -168
========================================
- Hits 13227 7739 -5488
- Misses 5902 11222 +5320 |
ok, so you say that this PR does not have any impact on functional metrics results but improves performances, correct? |
Delete some D2H ops
if not
will be bring a sync within host and device, so replace it fromif not
totorch.where
Before submitting
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
before:
after:
Host will be block until device done, this D2H maybe block pipeline of host and device, so I use
where op
to replaceif not