From a74a869aa008dee06810c9958682f019fc0b3757 Mon Sep 17 00:00:00 2001 From: David Wendt Date: Mon, 17 Apr 2023 09:41:13 -0400 Subject: [PATCH 1/4] Fix gtests to always pass null-count to set_null_mask calls --- cpp/tests/column/column_test.cu | 7 +++++-- cpp/tests/column/column_view_shallow_test.cpp | 5 +++-- cpp/tests/groupby/rank_scan_tests.cpp | 3 ++- cpp/tests/reductions/rank_tests.cpp | 4 ++-- cpp/tests/rolling/grouped_rolling_range_test.cpp | 2 +- cpp/tests/transform/nans_to_null_test.cpp | 6 +++--- 6 files changed, 16 insertions(+), 11 deletions(-) diff --git a/cpp/tests/column/column_test.cu b/cpp/tests/column/column_test.cu index bba309fd8a7..6ae76f15f67 100644 --- a/cpp/tests/column/column_test.cu +++ b/cpp/tests/column/column_test.cu @@ -157,7 +157,8 @@ TYPED_TEST(TypedColumnTest, SetEmptyNullMaskNonZeroNullCount) { cudf::column col{this->type(), this->num_elements(), std::move(this->data)}; rmm::device_buffer empty_null_mask{}; - EXPECT_THROW(col.set_null_mask(empty_null_mask, this->num_elements()), cudf::logic_error); + EXPECT_THROW(col.set_null_mask(std::move(empty_null_mask), this->num_elements()), + cudf::logic_error); } TYPED_TEST(TypedColumnTest, SetInvalidSizeNullMaskNonZeroNullCount) @@ -165,7 +166,9 @@ TYPED_TEST(TypedColumnTest, SetInvalidSizeNullMaskNonZeroNullCount) cudf::column col{this->type(), this->num_elements(), std::move(this->data)}; auto invalid_size_null_mask = create_null_mask(std::min(this->num_elements() - 50, 0), cudf::mask_state::ALL_VALID); - EXPECT_THROW(col.set_null_mask(invalid_size_null_mask, this->num_elements()), cudf::logic_error); + EXPECT_THROW( + col.set_null_mask(invalid_size_null_mask, this->num_elements(), cudf::get_default_stream()), + cudf::logic_error); } TYPED_TEST(TypedColumnTest, SetNullCountEmptyMask) diff --git a/cpp/tests/column/column_view_shallow_test.cpp b/cpp/tests/column/column_view_shallow_test.cpp index 4d8f06db677..5704db34a9b 100644 --- a/cpp/tests/column/column_view_shallow_test.cpp +++ b/cpp/tests/column/column_view_shallow_test.cpp @@ -261,7 +261,8 @@ TYPED_TEST(ColumnViewShallowTests, shallow_hash_mutable) { if constexpr (cudf::is_nested()) { col->child(0).set_null_mask( - cudf::create_null_mask(col->child(0).size(), cudf::mask_state::ALL_NULL)); + cudf::create_null_mask(col->child(0).size(), cudf::mask_state::ALL_NULL), + col->child(0).size()); auto col_child_updated = cudf::mutable_column_view{*col}; EXPECT_NE(shallow_hash(col_view), shallow_hash(col_child_updated)); } @@ -427,7 +428,7 @@ TYPED_TEST(ColumnViewShallowTests, is_shallow_equivalent_mutable) { if constexpr (cudf::is_nested()) { col->child(0).set_null_mask( - cudf::create_null_mask(col->child(0).size(), cudf::mask_state::ALL_NULL)); + cudf::create_null_mask(col->child(0).size(), cudf::mask_state::ALL_NULL), col->size()); auto col_child_updated = cudf::mutable_column_view{*col}; EXPECT_FALSE(is_shallow_equivalent(col_view, col_child_updated)); } diff --git a/cpp/tests/groupby/rank_scan_tests.cpp b/cpp/tests/groupby/rank_scan_tests.cpp index a803e8ebfc6..b4539b16a7e 100644 --- a/cpp/tests/groupby/rank_scan_tests.cpp +++ b/cpp/tests/groupby/rank_scan_tests.cpp @@ -354,7 +354,8 @@ TYPED_TEST(typed_groupby_rank_scan_test, structsWithNullPushdown) auto const definitely_null_structs = [&] { auto struct_column = get_struct_column(); - struct_column->set_null_mask(cudf::create_null_mask(num_rows, cudf::mask_state::ALL_NULL)); + struct_column->set_null_mask(cudf::create_null_mask(num_rows, cudf::mask_state::ALL_NULL), + struct_column->size()); return struct_column; }(); diff --git a/cpp/tests/reductions/rank_tests.cpp b/cpp/tests/reductions/rank_tests.cpp index 9cbaba91d2f..db357cdee8b 100644 --- a/cpp/tests/reductions/rank_tests.cpp +++ b/cpp/tests/reductions/rank_tests.cpp @@ -214,8 +214,8 @@ TYPED_TEST(TypedRankScanTest, StructsWithNullPushdown) // First, verify that if the structs column has only nulls, all output rows are ranked 1. { - struct_col->set_null_mask( - create_null_mask(12, cudf::mask_state::ALL_NULL)); // Null mask not pushed down to members. + // Null mask not pushed down to members. + struct_col->set_null_mask(create_null_mask(12, cudf::mask_state::ALL_NULL), struct_col->size()); auto const expected_null_result = rank_result_col{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}; auto const expected_percent_rank_null_result = percent_result_col{0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0}; diff --git a/cpp/tests/rolling/grouped_rolling_range_test.cpp b/cpp/tests/rolling/grouped_rolling_range_test.cpp index 33310956cbc..57ad103b165 100644 --- a/cpp/tests/rolling/grouped_rolling_range_test.cpp +++ b/cpp/tests/rolling/grouped_rolling_range_test.cpp @@ -167,7 +167,7 @@ struct GroupedRollingRangeOrderByDecimalTypedTest : BaseGroupedRollingRangeOrder 10, 12, false); // Nulls in third group. - col->set_null_mask(std::move(new_null_mask)); + col->set_null_mask(std::move(new_null_mask), 6); return col; }(); diff --git a/cpp/tests/transform/nans_to_null_test.cpp b/cpp/tests/transform/nans_to_null_test.cpp index 9590c5848f7..57eeb793720 100644 --- a/cpp/tests/transform/nans_to_null_test.cpp +++ b/cpp/tests/transform/nans_to_null_test.cpp @@ -27,11 +27,11 @@ template struct NaNsToNullTest : public cudf::test::BaseFixture { void run_test(cudf::column_view const& input, cudf::column_view const& expected) { - auto got_mask = cudf::nans_to_nulls(input); + auto [null_mask, null_count] = cudf::nans_to_nulls(input); cudf::column got(input); - got.set_null_mask(std::move(*(got_mask.first))); + got.set_null_mask(std::move(*(null_mask)), null_count); - EXPECT_EQ(expected.null_count(), got_mask.second); + EXPECT_EQ(expected.null_count(), null_count); CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected, got.view()); } From 092794e8b64fe0a62b9187985b6e5b36dc835dd6 Mon Sep 17 00:00:00 2001 From: David Wendt Date: Mon, 17 Apr 2023 09:58:11 -0400 Subject: [PATCH 2/4] add scatter-lists fix for null-count --- cpp/include/cudf/lists/detail/scatter.cuh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/include/cudf/lists/detail/scatter.cuh b/cpp/include/cudf/lists/detail/scatter.cuh index 856914b445e..18cb147d1e4 100644 --- a/cpp/include/cudf/lists/detail/scatter.cuh +++ b/cpp/include/cudf/lists/detail/scatter.cuh @@ -141,7 +141,7 @@ std::unique_ptr scatter_impl(rmm::device_uvector cons target.size(), rmm::device_buffer{}, std::move(null_mask), - cudf::UNKNOWN_NULL_COUNT, + target.null_count(), std::move(children)); } From 2def90de5316c5547eab3bde10f97f6aae6e3461 Mon Sep 17 00:00:00 2001 From: David Wendt Date: Mon, 17 Apr 2023 10:33:11 -0400 Subject: [PATCH 3/4] additional fixes missed on first pass --- cpp/tests/column/column_view_shallow_test.cpp | 4 ++-- cpp/tests/groupby/rank_scan_tests.cpp | 2 +- cpp/tests/reductions/rank_tests.cpp | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cpp/tests/column/column_view_shallow_test.cpp b/cpp/tests/column/column_view_shallow_test.cpp index 5704db34a9b..3e5650652e1 100644 --- a/cpp/tests/column/column_view_shallow_test.cpp +++ b/cpp/tests/column/column_view_shallow_test.cpp @@ -159,7 +159,7 @@ TYPED_TEST(ColumnViewShallowTests, shallow_hash_update_data) } // add null_mask + new column_view = diff hash. { - col->set_null_mask(cudf::create_null_mask(col->size(), cudf::mask_state::ALL_VALID)); + col->set_null_mask(cudf::create_null_mask(col->size(), cudf::mask_state::ALL_VALID), 0); auto col_view_new = cudf::column_view{*col}; EXPECT_NE(shallow_hash(col_view), shallow_hash(col_view_new)); [[maybe_unused]] auto const nulls = col_view_new.null_count(); @@ -326,7 +326,7 @@ TYPED_TEST(ColumnViewShallowTests, is_shallow_equivalent_update_data) } // add null_mask + new column_view = diff hash. { - col->set_null_mask(cudf::create_null_mask(col->size(), cudf::mask_state::ALL_VALID)); + col->set_null_mask(cudf::create_null_mask(col->size(), cudf::mask_state::ALL_VALID), 0); auto col_view_new = cudf::column_view{*col}; EXPECT_FALSE(is_shallow_equivalent(col_view, col_view_new)); [[maybe_unused]] auto const nulls = col_view_new.null_count(); diff --git a/cpp/tests/groupby/rank_scan_tests.cpp b/cpp/tests/groupby/rank_scan_tests.cpp index b4539b16a7e..76b05566e4d 100644 --- a/cpp/tests/groupby/rank_scan_tests.cpp +++ b/cpp/tests/groupby/rank_scan_tests.cpp @@ -355,7 +355,7 @@ TYPED_TEST(typed_groupby_rank_scan_test, structsWithNullPushdown) auto const definitely_null_structs = [&] { auto struct_column = get_struct_column(); struct_column->set_null_mask(cudf::create_null_mask(num_rows, cudf::mask_state::ALL_NULL), - struct_column->size()); + num_rows); return struct_column; }(); diff --git a/cpp/tests/reductions/rank_tests.cpp b/cpp/tests/reductions/rank_tests.cpp index db357cdee8b..4cc8d7e6600 100644 --- a/cpp/tests/reductions/rank_tests.cpp +++ b/cpp/tests/reductions/rank_tests.cpp @@ -215,7 +215,8 @@ TYPED_TEST(TypedRankScanTest, StructsWithNullPushdown) // First, verify that if the structs column has only nulls, all output rows are ranked 1. { // Null mask not pushed down to members. - struct_col->set_null_mask(create_null_mask(12, cudf::mask_state::ALL_NULL), struct_col->size()); + struct_col->set_null_mask(create_null_mask(struct_col->size(), cudf::mask_state::ALL_NULL), + struct_col->size()); auto const expected_null_result = rank_result_col{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}; auto const expected_percent_rank_null_result = percent_result_col{0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0}; From f3aee709d828a70ea2fbb0f12b7ef2800615fe37 Mon Sep 17 00:00:00 2001 From: David Wendt Date: Mon, 17 Apr 2023 15:11:38 -0400 Subject: [PATCH 4/4] remove extra parens --- cpp/tests/transform/nans_to_null_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/tests/transform/nans_to_null_test.cpp b/cpp/tests/transform/nans_to_null_test.cpp index 57eeb793720..2de06641c7f 100644 --- a/cpp/tests/transform/nans_to_null_test.cpp +++ b/cpp/tests/transform/nans_to_null_test.cpp @@ -29,7 +29,7 @@ struct NaNsToNullTest : public cudf::test::BaseFixture { { auto [null_mask, null_count] = cudf::nans_to_nulls(input); cudf::column got(input); - got.set_null_mask(std::move(*(null_mask)), null_count); + got.set_null_mask(std::move(*null_mask), null_count); EXPECT_EQ(expected.null_count(), null_count);