-
Notifications
You must be signed in to change notification settings - Fork 915
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 assert_column_memory_*
#9882
Add assert_column_memory_*
#9882
Conversation
Codecov Report
@@ Coverage Diff @@
## branch-22.04 #9882 +/- ##
=============================================
Coverage 10.50% 10.50%
=============================================
Files 126 127 +1
Lines 21218 21215 -3
=============================================
Hits 2228 2228
+ Misses 18990 18987 -3
Continue to review full report at Codecov.
|
This PR has been labeled |
python/cudf/cudf/testing/_utils.py
Outdated
def assert_column_memory_eq( | ||
lhs: cudf.core.column.ColumnBase, rhs: cudf.core.column.ColumnBase | ||
): | ||
assert lhs.base_data_ptr == rhs.base_data_ptr | ||
assert lhs.base_mask_ptr == rhs.base_mask_ptr | ||
for lhs_child, rhs_child in zip(lhs.base_children, rhs.base_children): | ||
assert_column_memory_eq(lhs_child, rhs_child) |
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.
Wouldn't this also return True
for columns that have the same base pointers but different sizes?
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.
Right.. Is adding check for size sufficient?
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 - an additional check for size
should be enough.
I'm wondering.. is it possible for two columns locating on different nodes in a multi-node setup to have the same memory address but are in fact two separate objects? cc @shwina |
Probably? But I don't think this API should be concerned with that scenario. |
Moved this over to the 22.04 project board. Please retarget when you have a chance :) |
rerun tests |
This PR has been labeled |
…ent/assert_column_memory_equal
…ent/assert_column_memory_equal
@gpucibot merge |
This PR refactors
test_index.py::test_index_copy_deep
and adds test utility functionassert_column_memory_eq
andassert_column_memory_ne
, which tests if two cudf columns possess (or not possess) the same piece of memory.