-
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
get_device_memory_objects(): dispatch on cudf.core.frame.Frame #658
get_device_memory_objects(): dispatch on cudf.core.frame.Frame #658
Conversation
Current CI fail relates to rapidsai/cudf#8570 |
@charlesbluca can you take a look at this ? |
Sure! @madsbk do you have a link to the test failure? I am only seeing passed and pending checks on my end |
rerun tests |
It looks like things are failing in distributed tests:
These test pass locally so. I'll rerun now |
rerun tests |
Codecov Report
@@ Coverage Diff @@
## branch-21.08 #658 +/- ##
===============================================
Coverage ? 61.75%
===============================================
Files ? 21
Lines ? 2604
Branches ? 0
===============================================
Hits ? 1608
Misses ? 996
Partials ? 0 Continue to review full report at Codecov.
|
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.
Thanks @madsbk
@gpucibot merge |
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.
approved with a suggestion 👍
py.test --cache-clear -vs distributed/distributed/protocol/tests/test_cupy.py | ||
py.test --cache-clear -vs distributed/distributed/protocol/tests/test_numba.py | ||
py.test --cache-clear -vs distributed/distributed/protocol/tests/test_rmm.py | ||
py.test --cache-clear -vs distributed/distributed/protocol/tests/test_collection_cuda.py | ||
py.test --cache-clear -vs distributed/distributed/tests/test_nanny.py | ||
py.test --cache-clear -vs distributed/distributed/diagnostics/tests/test_nvml.py |
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.
py.test --cache-clear -vs distributed/distributed/protocol/tests/test_cupy.py | |
py.test --cache-clear -vs distributed/distributed/protocol/tests/test_numba.py | |
py.test --cache-clear -vs distributed/distributed/protocol/tests/test_rmm.py | |
py.test --cache-clear -vs distributed/distributed/protocol/tests/test_collection_cuda.py | |
py.test --cache-clear -vs distributed/distributed/tests/test_nanny.py | |
py.test --cache-clear -vs distributed/distributed/diagnostics/tests/test_nvml.py | |
pytest --cache-clear -vs distributed/distributed/protocol/tests/test_cupy.py | |
pytest --cache-clear -vs distributed/distributed/protocol/tests/test_numba.py | |
pytest --cache-clear -vs distributed/distributed/protocol/tests/test_rmm.py | |
pytest --cache-clear -vs distributed/distributed/protocol/tests/test_collection_cuda.py | |
pytest --cache-clear -vs distributed/distributed/tests/test_nanny.py | |
pytest --cache-clear -vs distributed/distributed/diagnostics/tests/test_nvml.py |
While we're touching these lines, can we update these commands to pytest
instead of py.test
? It seems that py.test
may be deprecated according to the SO discussion below.
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.
whoops. forgot that @gpucibot merge
would merge after my approval. this can be addressed in a future PR I guess.
Addresses @ajschmidt8 comments in #658 (comment) Authors: - Benjamin Zaitlen (https://github.com/quasiben) Approvers: - Mads R. B. Kristensen (https://github.com/madsbk) - AJ Schmidt (https://github.com/ajschmidt8) URL: #661
get_device_memory_objects()
now dispatch oncudf.core.frame.Frame
instead ofcudf.core.dataframe.DataFrame
only.This includes the CI fix, closes #659