Skip to content

Commit

Permalink
Fix some libcudf functions to set the null count on returning columns (
Browse files Browse the repository at this point in the history
…#13331)

Fixes a few more places where the null count was not being set in a libcudf function before returning a column. A couple of places were setting the null count value into the mutable-view object. So the value was calculated correctly but not set into the actual column object.

Reference: #11968

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

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

URL: #13331
  • Loading branch information
davidwendt authored May 15, 2023
1 parent b16f9c4 commit 4fe3e38
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 1 deletion.
3 changes: 3 additions & 0 deletions cpp/src/binaryop/binaryop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ std::unique_ptr<column> binary_operation(LhsType const& lhs,

auto out_view = out->mutable_view();
cudf::binops::compiled::binary_operation(out_view, lhs, rhs, op, stream);
// TODO: consider having the binary_operation count nulls instead
out->set_null_count(cudf::detail::null_count(out_view.null_mask(), 0, out->size(), stream));
return out;
}
} // namespace compiled
Expand Down Expand Up @@ -373,6 +375,7 @@ std::unique_ptr<column> binary_operation(column_view const& lhs,

auto out_view = out->mutable_view();
binops::jit::binary_operation(out_view, lhs, rhs, ptx, stream);
out->set_null_count(cudf::detail::null_count(out_view.null_mask(), 0, out->size(), stream));
return out;
}
} // namespace detail
Expand Down
3 changes: 2 additions & 1 deletion cpp/src/copying/copy_range.cu
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 @@ -108,6 +108,7 @@ struct out_of_place_copy_range_dispatch {
if (source_end != source_begin) { // otherwise no-op
auto ret_view = p_ret->mutable_view();
in_place_copy_range<T>(source, ret_view, source_begin, source_end, target_begin, stream);
p_ret->set_null_count(ret_view.null_count());
}

return p_ret;
Expand Down
1 change: 1 addition & 0 deletions cpp/src/filling/fill.cu
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ struct out_of_place_fill_range_dispatch {
auto ret_view = p_ret->mutable_view();
using DeviceType = cudf::device_storage_type_t<T>;
in_place_fill<DeviceType>(ret_view, begin, end, value, stream);
p_ret->set_null_count(ret_view.null_count());
}

return p_ret;
Expand Down
6 changes: 6 additions & 0 deletions cpp/tests/copying/purge_nonempty_nulls_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ TEST_F(PurgeNonEmptyNullsTest, SingleLevelList)
// Set nullmask, post construction.
cudf::detail::set_null_mask(
input->mutable_view().null_mask(), 2, 3, false, cudf::get_default_stream());
input->set_null_count(1);
EXPECT_TRUE(cudf::may_have_nonempty_nulls(*input));
EXPECT_TRUE(cudf::has_nonempty_nulls(*input));

Expand Down Expand Up @@ -158,6 +159,7 @@ TEST_F(PurgeNonEmptyNullsTest, TwoLevelList)
// Set nullmask, post construction.
cudf::detail::set_null_mask(
input->mutable_view().null_mask(), 3, 4, false, cudf::get_default_stream());
input->set_null_count(1);
EXPECT_TRUE(cudf::may_have_nonempty_nulls(*input));
EXPECT_TRUE(cudf::has_nonempty_nulls(*input));

Expand Down Expand Up @@ -213,6 +215,7 @@ TEST_F(PurgeNonEmptyNullsTest, ThreeLevelList)
// Set nullmask, post construction.
cudf::detail::set_null_mask(
input->mutable_view().null_mask(), 3, 4, false, cudf::get_default_stream());
input->set_null_count(1);
EXPECT_TRUE(cudf::may_have_nonempty_nulls(*input));
EXPECT_TRUE(cudf::has_nonempty_nulls(*input));

Expand Down Expand Up @@ -267,6 +270,7 @@ TEST_F(PurgeNonEmptyNullsTest, ListOfStrings)
// Set nullmask, post construction.
cudf::detail::set_null_mask(
input->mutable_view().null_mask(), 2, 3, false, cudf::get_default_stream());
input->set_null_count(1);
EXPECT_TRUE(cudf::may_have_nonempty_nulls(*input));
EXPECT_TRUE(cudf::has_nonempty_nulls(*input));

Expand Down Expand Up @@ -332,6 +336,7 @@ TEST_F(PurgeNonEmptyNullsTest, UnsanitizedListOfUnsanitizedStrings)

// Set strings nullmask, post construction.
cudf::set_null_mask(strings->mutable_view().null_mask(), 7, 8, false);
strings->set_null_count(1);
EXPECT_TRUE(cudf::may_have_nonempty_nulls(*strings));
EXPECT_TRUE(cudf::has_nonempty_nulls(*strings));

Expand All @@ -358,6 +363,7 @@ TEST_F(PurgeNonEmptyNullsTest, UnsanitizedListOfUnsanitizedStrings)
// Set lists nullmask, post construction.
cudf::detail::set_null_mask(
lists->mutable_view().null_mask(), 2, 3, false, cudf::get_default_stream());
lists->set_null_count(1);
EXPECT_TRUE(cudf::may_have_nonempty_nulls(*lists));
EXPECT_TRUE(cudf::has_nonempty_nulls(*lists));

Expand Down

0 comments on commit 4fe3e38

Please sign in to comment.