-
Notifications
You must be signed in to change notification settings - Fork 96
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
Add device synchonization for local CuPy benchmarks with Dask profiling #533
Add device synchonization for local CuPy benchmarks with Dask profiling #533
Conversation
cc @pentschev |
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, thanks @charlesbluca !
Just FYI on labels @charlesbluca , you need at least one of each:
Otherwise the label checker won't pass. |
Woohoo! Thanks @charlesbluca ! |
Are you able to select labels Charles? If not, please let us know |
No, I don't think I am - do I need additional repo privileges for that? |
Codecov Report
@@ Coverage Diff @@
## branch-0.18 #533 +/- ##
===============================================
+ Coverage 90.42% 92.45% +2.02%
===============================================
Files 15 16 +1
Lines 1128 1550 +422
===============================================
+ Hits 1020 1433 +413
- Misses 108 117 +9
Continue to review full report at Codecov.
|
I created a request on your behalf @charlesbluca . |
@gpucibot merge |
Thanks Peter :) |
@gpucibot merge |
This PR ensures CUDA device synchronization for local CuPy benchmarks when profiling with Dask performance reports.
Prior to this, the run times of benchmarks running over UCX would vary drastically depending on if a performance report was generated or not:
Additionally, I suppressed flake8 rule E402 in
docs/source/conf.py
, as that had been causing the pre-commit hooks to fail up until now. I'm okay with removing that change if it would be better suited for a separate PR.