Skip to content

Commit

Permalink
Fix libcudf gtests to pass null-count=0 for empty validity masks (#12923
Browse files Browse the repository at this point in the history
)

Removing some unneeded usages of `cudf::UNKNOWN_NULL_COUNT` specifically in libcudf gtests source files.
These were found when working on other PRs. 
Reducing the usage like this hopefully will move us closer to not using it at all.
There are few places in the gtest source files where we still use it even though the number of nulls is technically known but these may be fixed by adding and using some new test utilities in a future PR.

Authors:
  - David Wendt (https://github.com/davidwendt)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Nghia Truong (https://github.com/ttnghia)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #12923
  • Loading branch information
davidwendt authored Mar 10, 2023
1 parent 4da6b19 commit e591f68
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 97 deletions.
2 changes: 1 addition & 1 deletion cpp/benchmarks/common/generate_input.cu
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ std::unique_ptr<cudf::column> create_random_column<cudf::list_view>(data_profile
num_rows,
std::move(offsets_column),
std::move(current_child_column),
profile.get_null_probability().has_value() ? null_count : 0, // cudf::UNKNOWN_NULL_COUNT,
profile.get_null_probability().has_value() ? null_count : 0,
profile.get_null_probability().has_value() ? std::move(null_mask) : rmm::device_buffer{});
}
return list_column; // return the top-level column
Expand Down
20 changes: 6 additions & 14 deletions cpp/tests/copying/gather_struct_tests.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2022, NVIDIA CORPORATION.
* Copyright (c) 2020-2023, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -388,13 +388,9 @@ TYPED_TEST(TypedStructGatherTest, TestGatherStructOfListOfStructs)
auto const struct_of_list_of_structs = [&] {
auto numeric_column =
numerics<TypeParam>{{5, 10, 15, 20, 25, 30, 35, 45, 50, 55, 60, 65, 70, 75}};
auto structs_column = structs{{numeric_column}}.release();
auto list_of_structs_column =
cudf::make_lists_column(7,
offsets{0, 2, 4, 6, 8, 10, 12, 14}.release(),
std::move(structs_column),
cudf::UNKNOWN_NULL_COUNT,
{});
auto structs_column = structs{{numeric_column}}.release();
auto list_of_structs_column = cudf::make_lists_column(
7, offsets{0, 2, 4, 6, 8, 10, 12, 14}.release(), std::move(structs_column), 0, {});

std::vector<std::unique_ptr<cudf::column>> vector_of_columns;
vector_of_columns.push_back(std::move(list_of_structs_column));
Expand All @@ -410,12 +406,8 @@ TYPED_TEST(TypedStructGatherTest, TestGatherStructOfListOfStructs)
auto expected_gather_result = [&] {
auto expected_numeric_col = numerics<TypeParam>{{70, 75, 50, 55, 35, 45, 25, 30, 15, 20}};
auto expected_struct_col = structs{{expected_numeric_col}}.release();
auto expected_list_of_structs_column =
cudf::make_lists_column(5,
offsets{0, 2, 4, 6, 8, 10}.release(),
std::move(expected_struct_col),
cudf::UNKNOWN_NULL_COUNT,
{});
auto expected_list_of_structs_column = cudf::make_lists_column(
5, offsets{0, 2, 4, 6, 8, 10}.release(), std::move(expected_struct_col), 0, {});
std::vector<std::unique_ptr<cudf::column>> expected_vector_of_columns;
expected_vector_of_columns.push_back(std::move(expected_list_of_structs_column));
return structs{std::move(expected_vector_of_columns), {0, 1, 1, 1, 1}};
Expand Down
35 changes: 10 additions & 25 deletions cpp/tests/io/parquet_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1313,11 +1313,8 @@ TEST_F(ParquetWriterTest, ListOfStruct)
cudf::test::fixed_width_column_wrapper<cudf::size_type>{0, 2, 5, 5, 6}.release();
auto num_list_rows = list_offsets_column->size() - 1;

auto list_col = cudf::make_lists_column(num_list_rows,
std::move(list_offsets_column),
std::move(struct_2),
cudf::UNKNOWN_NULL_COUNT,
{});
auto list_col = cudf::make_lists_column(
num_list_rows, std::move(list_offsets_column), std::move(struct_2), 0, {});

auto expected = table_view({*list_col});

Expand Down Expand Up @@ -1779,11 +1776,8 @@ TEST_F(ParquetChunkedWriterTest, ListOfStruct)
cudf::test::fixed_width_column_wrapper<cudf::size_type>{0, 2, 3, 3}.release();
auto num_list_rows_1 = list_offsets_column_1->size() - 1;

auto list_col_1 = cudf::make_lists_column(num_list_rows_1,
std::move(list_offsets_column_1),
struct_2_1.release(),
cudf::UNKNOWN_NULL_COUNT,
{});
auto list_col_1 = cudf::make_lists_column(
num_list_rows_1, std::move(list_offsets_column_1), struct_2_1.release(), 0, {});

auto table_1 = table_view({*list_col_1});

Expand All @@ -1798,11 +1792,8 @@ TEST_F(ParquetChunkedWriterTest, ListOfStruct)
cudf::test::fixed_width_column_wrapper<cudf::size_type>{0, 1, 2, 3}.release();
auto num_list_rows_2 = list_offsets_column_2->size() - 1;

auto list_col_2 = cudf::make_lists_column(num_list_rows_2,
std::move(list_offsets_column_2),
struct_2_2.release(),
cudf::UNKNOWN_NULL_COUNT,
{});
auto list_col_2 = cudf::make_lists_column(
num_list_rows_2, std::move(list_offsets_column_2), struct_2_2.release(), 0, {});

auto table_2 = table_view({*list_col_2});

Expand Down Expand Up @@ -1861,11 +1852,8 @@ TEST_F(ParquetChunkedWriterTest, ListOfStructOfStructOfListOfList)
cudf::test::fixed_width_column_wrapper<cudf::size_type>{0, 2, 3, 4}.release();
auto num_list_rows_1 = list_offsets_column_1->size() - 1;

auto list_col_1 = cudf::make_lists_column(num_list_rows_1,
std::move(list_offsets_column_1),
struct_2_1.release(),
cudf::UNKNOWN_NULL_COUNT,
{});
auto list_col_1 = cudf::make_lists_column(
num_list_rows_1, std::move(list_offsets_column_1), struct_2_1.release(), 0, {});

auto table_1 = table_view({*list_col_1});

Expand All @@ -1889,11 +1877,8 @@ TEST_F(ParquetChunkedWriterTest, ListOfStructOfStructOfListOfList)
cudf::test::fixed_width_column_wrapper<cudf::size_type>{0, 1, 2}.release();
auto num_list_rows_2 = list_offsets_column_2->size() - 1;

auto list_col_2 = cudf::make_lists_column(num_list_rows_2,
std::move(list_offsets_column_2),
struct_2_2.release(),
cudf::UNKNOWN_NULL_COUNT,
{});
auto list_col_2 = cudf::make_lists_column(
num_list_rows_2, std::move(list_offsets_column_2), struct_2_2.release(), 0, {});

auto table_2 = table_view({*list_col_2});

Expand Down
10 changes: 5 additions & 5 deletions cpp/tests/lists/explode_tests.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021-2022, NVIDIA CORPORATION.
* Copyright (c) 2021-2023, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -469,8 +469,8 @@ TYPED_TEST(ExplodeTypedTest, ListOfStructs)
cudf::test::strings_column_wrapper string_col{
"70", "75", "50", "55", "35", "45", "25", "30", "15", "20"};
auto struct_col = cudf::test::structs_column_wrapper{{numeric_col, string_col}}.release();
auto a = cudf::make_lists_column(
5, FCW{0, 2, 4, 6, 8, 10}.release(), std::move(struct_col), cudf::UNKNOWN_NULL_COUNT, {});
auto a =
cudf::make_lists_column(5, FCW{0, 2, 4, 6, 8, 10}.release(), std::move(struct_col), 0, {});

FCW b{100, 200, 300, 400, 500};

Expand Down Expand Up @@ -1118,8 +1118,8 @@ TYPED_TEST(ExplodeOuterTypedTest, ListOfStructs)
cudf::test::strings_column_wrapper string_col{
"70", "75", "50", "55", "35", "45", "25", "30", "15", "20"};
auto struct_col = cudf::test::structs_column_wrapper{{numeric_col, string_col}}.release();
auto a = cudf::make_lists_column(
5, FCW{0, 2, 4, 6, 8, 10}.release(), std::move(struct_col), cudf::UNKNOWN_NULL_COUNT, {});
auto a =
cudf::make_lists_column(5, FCW{0, 2, 4, 6, 8, 10}.release(), std::move(struct_col), 0, {});

FCW b{100, 200, 300, 400, 500};

Expand Down
13 changes: 6 additions & 7 deletions cpp/tests/reductions/tdigest_tests.cu
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, NVIDIA CORPORATION.
* Copyright (c) 2022-2023, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -97,8 +97,8 @@ TEST_F(ReductionTDigestMerge, FewHeavyCentroids)
cudf::test::fixed_width_column_wrapper<double> c0w{100.0, 50.0};
cudf::test::structs_column_wrapper c0s({c0c, c0w});
cudf::test::fixed_width_column_wrapper<cudf::offset_type> c0_offsets{0, 2};
auto c0l = cudf::make_lists_column(
1, c0_offsets.release(), c0s.release(), cudf::UNKNOWN_NULL_COUNT, rmm::device_buffer{});
auto c0l =
cudf::make_lists_column(1, c0_offsets.release(), c0s.release(), 0, rmm::device_buffer{});
cudf::test::fixed_width_column_wrapper<double> c0min{1.0};
cudf::test::fixed_width_column_wrapper<double> c0max{2.0};
std::vector<std::unique_ptr<cudf::column>> c0_children;
Expand All @@ -114,8 +114,8 @@ TEST_F(ReductionTDigestMerge, FewHeavyCentroids)
cudf::test::fixed_width_column_wrapper<double> c1w{200.0, 50.0};
cudf::test::structs_column_wrapper c1s({c1c, c1w});
cudf::test::fixed_width_column_wrapper<cudf::offset_type> c1_offsets{0, 2};
auto c1l = cudf::make_lists_column(
1, c1_offsets.release(), c1s.release(), cudf::UNKNOWN_NULL_COUNT, rmm::device_buffer{});
auto c1l =
cudf::make_lists_column(1, c1_offsets.release(), c1s.release(), 0, rmm::device_buffer{});
cudf::test::fixed_width_column_wrapper<double> c1min{3.0};
cudf::test::fixed_width_column_wrapper<double> c1max{4.0};
std::vector<std::unique_ptr<cudf::column>> c1_children;
Expand Down Expand Up @@ -150,8 +150,7 @@ TEST_F(ReductionTDigestMerge, FewHeavyCentroids)
cudf::test::fixed_width_column_wrapper<double> ew{100.0, 50.0, 200.0, 50.0};
cudf::test::structs_column_wrapper es({ec, ew});
cudf::test::fixed_width_column_wrapper<cudf::offset_type> e_offsets{0, 4};
auto el = cudf::make_lists_column(
1, e_offsets.release(), es.release(), cudf::UNKNOWN_NULL_COUNT, rmm::device_buffer{});
auto el = cudf::make_lists_column(1, e_offsets.release(), es.release(), 0, rmm::device_buffer{});
cudf::test::fixed_width_column_wrapper<double> emin{1.0};
cudf::test::fixed_width_column_wrapper<double> emax{4.0};
std::vector<std::unique_ptr<cudf::column>> e_children;
Expand Down
12 changes: 6 additions & 6 deletions cpp/tests/sort/sort_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1053,9 +1053,9 @@ TEST_F(SortCornerTest, WithEmptyStructColumn)

// struct{}, int, int
int_col col_for_mask{{0, 0, 0, 0, 0, 0}, {1, 0, 1, 1, 1, 1}};
auto null_mask = cudf::copy_bitmask(col_for_mask.release()->view());
auto struct_col =
cudf::make_structs_column(6, {}, cudf::UNKNOWN_NULL_COUNT, std::move(null_mask));
auto null_mask = cudf::copy_bitmask(col_for_mask);
auto struct_col = cudf::make_structs_column(
6, {}, cudf::column_view(col_for_mask).null_count(), std::move(null_mask));

int_col col1{{1, 2, 3, 1, 2, 3}};
int_col col2{{1, 1, 1, 2, 2, 2}};
Expand All @@ -1082,10 +1082,10 @@ TEST_F(SortCornerTest, WithEmptyStructColumn)

// struct{struct{}, struct{int}}
int_col col_for_mask2{{0, 0, 0, 0, 0, 0}, {1, 0, 1, 1, 0, 1}};
auto null_mask2 = cudf::copy_bitmask(col_for_mask2.release()->view());
auto null_mask2 = cudf::copy_bitmask(col_for_mask2);
std::vector<std::unique_ptr<cudf::column>> child_columns2;
auto child_col_1 =
cudf::make_structs_column(6, {}, cudf::UNKNOWN_NULL_COUNT, std::move(null_mask2));
auto child_col_1 = cudf::make_structs_column(
6, {}, cudf::column_view(col_for_mask2).null_count(), std::move(null_mask2));
child_columns2.push_back(std::move(child_col_1));
int_col col4{{5, 4, 3, 2, 1, 0}};
std::vector<std::unique_ptr<cudf::column>> grand_child;
Expand Down
6 changes: 3 additions & 3 deletions cpp/tests/stream_compaction/apply_boolean_mask_tests.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2022, NVIDIA CORPORATION.
* Copyright (c) 2019-2023, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -318,7 +318,7 @@ TEST_F(ApplyBooleanMask, ListOfStructsFiltering)
cudf::make_lists_column(5,
fixed_width_column_wrapper<int32_t>{0, 2, 4, 6, 8, 10}.release(),
struct_column.release(),
cudf::UNKNOWN_NULL_COUNT,
0,
{});

auto filter_mask = fixed_width_column_wrapper<bool>{{1, 0, 1, 0, 1}};
Expand All @@ -340,7 +340,7 @@ TEST_F(ApplyBooleanMask, ListOfStructsFiltering)
cudf::make_lists_column(3,
fixed_width_column_wrapper<int32_t>{0, 2, 4, 6}.release(),
expected_struct_column.release(),
cudf::UNKNOWN_NULL_COUNT,
0,
{});

CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(filtered_list_column,
Expand Down
22 changes: 6 additions & 16 deletions cpp/tests/structs/structs_column_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -422,11 +422,8 @@ TYPED_TEST(TypedStructColumnWrapperTest, TestListsOfStructs)
cudf::test::fixed_width_column_wrapper<size_type>{0, 2, 3, 5, 6}.release();
auto num_list_rows = list_offsets_column->size() - 1;

auto list_col = cudf::make_lists_column(num_list_rows,
std::move(list_offsets_column),
std::move(struct_col),
cudf::UNKNOWN_NULL_COUNT,
{});
auto list_col = cudf::make_lists_column(
num_list_rows, std::move(list_offsets_column), std::move(struct_col), 0, {});

// List of structs was constructed successfully. No exceptions.
// Verify that child columns is as it was set.
Expand Down Expand Up @@ -552,12 +549,8 @@ TYPED_TEST(TypedStructColumnWrapperTest, EmptyColumnsOfStructs)
EXPECT_TRUE(struct_column->size() == 0);
EXPECT_TRUE(struct_column->null_count() == 0);

auto empty_list_of_structs =
cudf::make_lists_column(0,
fixed_width_column_wrapper<size_type>{0}.release(),
std::move(struct_column),
cudf::UNKNOWN_NULL_COUNT,
{});
auto empty_list_of_structs = cudf::make_lists_column(
0, fixed_width_column_wrapper<size_type>{0}.release(), std::move(struct_column), 0, {});

EXPECT_TRUE(empty_list_of_structs->size() == 0);
EXPECT_TRUE(empty_list_of_structs->null_count() == 0);
Expand Down Expand Up @@ -613,11 +606,8 @@ TYPED_TEST(TypedStructColumnWrapperTest, CopyColumnFromView)
CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(clone_structs_column, structs_column);

auto list_of_structs_column =
cudf::make_lists_column(3,
fixed_width_column_wrapper<int32_t>{0, 2, 4, 6}.release(),
structs_column.release(),
cudf::UNKNOWN_NULL_COUNT,
{})
cudf::make_lists_column(
3, fixed_width_column_wrapper<int32_t>{0, 2, 4, 6}.release(), structs_column.release(), 0, {})
.release();

CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(list_of_structs_column->view(),
Expand Down
8 changes: 4 additions & 4 deletions cpp/tests/transform/row_bit_count_test.cu
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021-2022, NVIDIA CORPORATION.
* Copyright (c) 2021-2023, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -397,7 +397,7 @@ std::unique_ptr<cudf::column> build_nested_column1(std::vector<bool> const& stru
return cudf::make_lists_column(static_cast<cudf::size_type>(size),
outer_offsets_col.release(),
struct_column.release(),
cudf::UNKNOWN_NULL_COUNT,
0,
rmm::device_buffer{});
}

Expand Down Expand Up @@ -429,7 +429,7 @@ std::unique_ptr<cudf::column> build_nested_column2(std::vector<bool> const& stru
return make_lists_column(static_cast<cudf::size_type>(size),
outer_offsets_col.release(),
outer_struct.release(),
cudf::UNKNOWN_NULL_COUNT,
0,
rmm::device_buffer{});
}

Expand Down Expand Up @@ -514,7 +514,7 @@ TEST_F(RowBitCount, NestedTypes)
auto l4 = cudf::make_lists_column(static_cast<cudf::size_type>(l4_size),
l4_offsets_col.release(),
innermost_struct.release(),
cudf::UNKNOWN_NULL_COUNT,
0,
rmm::device_buffer{});

// inner struct
Expand Down
25 changes: 9 additions & 16 deletions cpp/tests/utilities_tests/lists_column_wrapper_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1380,8 +1380,8 @@ TYPED_TEST(ListColumnWrapperTestTyped, ListsOfStructs)

auto lists_column_offsets = test::fixed_width_column_wrapper<size_type>{0, 2, 4, 8}.release();
auto num_lists = lists_column_offsets->size() - 1;
auto lists_column = make_lists_column(
num_lists, std::move(lists_column_offsets), std::move(struct_column), UNKNOWN_NULL_COUNT, {});
auto lists_column =
make_lists_column(num_lists, std::move(lists_column_offsets), std::move(struct_column), 0, {});

// Check if child column is unchanged.

Expand Down Expand Up @@ -1444,18 +1444,14 @@ TYPED_TEST(ListColumnWrapperTestTyped, ListsOfListsOfStructs)

auto lists_column_offsets = test::fixed_width_column_wrapper<size_type>{0, 2, 4, 8}.release();
auto num_lists = lists_column_offsets->size() - 1;
auto lists_column = make_lists_column(
num_lists, std::move(lists_column_offsets), std::move(struct_column), UNKNOWN_NULL_COUNT, {});
auto lists_column =
make_lists_column(num_lists, std::move(lists_column_offsets), std::move(struct_column), 0, {});

auto lists_of_lists_column_offsets =
test::fixed_width_column_wrapper<size_type>{0, 2, 3}.release();
auto num_lists_of_lists = lists_of_lists_column_offsets->size() - 1;
auto lists_of_lists_of_structs_column =
make_lists_column(num_lists_of_lists,
std::move(lists_of_lists_column_offsets),
std::move(lists_column),
UNKNOWN_NULL_COUNT,
{});
auto num_lists_of_lists = lists_of_lists_column_offsets->size() - 1;
auto lists_of_lists_of_structs_column = make_lists_column(
num_lists_of_lists, std::move(lists_of_lists_column_offsets), std::move(lists_column), 0, {});

// Check if child column is unchanged.

Expand Down Expand Up @@ -1555,11 +1551,8 @@ TYPED_TEST(ListColumnWrapperTestTyped, LargeListsOfStructsWithValidity)
auto list_offset_column = test::fixed_width_column_wrapper<size_type>(
list_offset_iterator, list_offset_iterator + num_list_rows + 1)
.release();
auto lists_column = make_lists_column(num_list_rows,
std::move(list_offset_column),
std::move(struct_column),
cudf::UNKNOWN_NULL_COUNT,
{});
auto lists_column = make_lists_column(
num_list_rows, std::move(list_offset_column), std::move(struct_column), 0, {});

// List construction succeeded.
// Verify that the child is unchanged.
Expand Down

0 comments on commit e591f68

Please sign in to comment.