Skip to content

Commit

Permalink
Fix libcudf to always pass null-count to set_null_mask (#13149)
Browse files Browse the repository at this point in the history
Fix calls to the `column::set_null_mask()` function to always pass the null-count value in libcudf source files.

Contributes to: #11968

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

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

URL: #13149
  • Loading branch information
davidwendt authored Apr 20, 2023
1 parent fc91b0f commit 9d36716
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 24 deletions.
15 changes: 7 additions & 8 deletions cpp/src/copying/shift.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 @@ -86,10 +86,9 @@ struct shift_functor {
cudf::strings_column_view(input), offset, fill_value, stream, mr);

if (input.nullable() || not fill_value.is_valid(stream)) {
auto const d_input = column_device_view::create(input, stream);
auto mask_pair = create_null_mask(*d_input, offset, fill_value, stream, mr);
output->set_null_mask(std::move(std::get<0>(mask_pair)));
output->set_null_count(std::get<1>(mask_pair));
auto const d_input = column_device_view::create(input, stream);
auto [null_mask, null_count] = create_null_mask(*d_input, offset, fill_value, stream, mr);
output->set_null_mask(std::move(null_mask), null_count);
}

return output;
Expand All @@ -114,9 +113,9 @@ struct shift_functor {
auto const scalar_is_valid = scalar.is_valid(stream);

if (input.nullable() || not scalar_is_valid) {
auto mask_pair = create_null_mask(*device_input, offset, fill_value, stream, mr);
output->set_null_mask(std::move(std::get<0>(mask_pair)));
output->set_null_count(std::get<1>(mask_pair));
auto [null_mask, null_count] =
create_null_mask(*device_input, offset, fill_value, stream, mr);
output->set_null_mask(std::move(null_mask), null_count);
}

auto const size = input.size();
Expand Down
3 changes: 2 additions & 1 deletion cpp/src/datetime/datetime_ops.cu
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,8 @@ std::unique_ptr<column> add_calendrical_months(column_view const& timestamp_colu
months_begin_iter,
stream,
mr);
output->set_null_mask(cudf::detail::copy_bitmask(timestamp_column, stream, mr));
output->set_null_mask(cudf::detail::copy_bitmask(timestamp_column, stream, mr),
timestamp_column.null_count());
return output;
} else {
return make_timestamp_column(
Expand Down
7 changes: 2 additions & 5 deletions cpp/src/groupby/sort/group_correlation.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 @@ -173,10 +173,7 @@ std::unique_ptr<column> group_covariance(column_view const& values_0,
};
auto [new_nullmask, null_count] =
cudf::detail::valid_if(count.begin<size_type>(), count.end<size_type>(), is_null, stream, mr);
if (null_count != 0) {
result->set_null_mask(std::move(new_nullmask));
result->set_null_count(null_count);
}
if (null_count != 0) { result->set_null_mask(std::move(new_nullmask), null_count); }
return result;
}

Expand Down
3 changes: 2 additions & 1 deletion cpp/src/groupby/sort/group_rank_scan.cu
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,6 @@ std::unique_ptr<column> group_rank_to_percentage(rank_method const method,
CUDF_EXPECTS(percentage != rank_percentage::NONE, "Percentage cannot be NONE");
auto ranks = make_fixed_width_column(
data_type{type_to_id<double>()}, group_labels.size(), mask_state::UNALLOCATED, stream, mr);
ranks->set_null_mask(copy_bitmask(rank, stream, mr));
auto mutable_ranks = ranks->mutable_view();

auto one_normalized = [] __device__(auto const rank, auto const group_size) {
Expand Down Expand Up @@ -321,6 +320,8 @@ std::unique_ptr<column> group_rank_to_percentage(rank_method const method,
: one_normalized(r, count);
});
}

ranks->set_null_count(0);
return ranks;
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/groupby/sort/group_scan_util.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ struct group_scan_functor<K, T, std::enable_if_t<is_group_scan_supported<K, T>()
make_null_replacement_iterator(*values_view, OpType::template identity<DeviceType>()),
thrust::identity<ResultDeviceType>{});
do_scan(input, result_view->begin<ResultDeviceType>(), OpType{});
result->set_null_mask(cudf::detail::copy_bitmask(values, stream, mr));
result->set_null_mask(cudf::detail::copy_bitmask(values, stream, mr), values.null_count());
} else {
auto input = thrust::make_transform_iterator(values_view->begin<DeviceType>(),
thrust::identity<ResultDeviceType>{});
Expand Down
3 changes: 2 additions & 1 deletion cpp/src/groupby/sort/scan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ void scan_result_functor::operator()<aggregation::RANK>(aggregation const& agg)
cudf::detail::scatter(table_view{{*result}}, *gather_map, table_view{{*result}}, stream, mr)
->release()[0]);
if (rank_agg._null_handling == null_policy::EXCLUDE) {
result->set_null_mask(cudf::detail::copy_bitmask(get_grouped_values(), stream, mr));
auto const values = get_grouped_values();
result->set_null_mask(cudf::detail::copy_bitmask(values, stream, mr), values.null_count());
}
cache.add_result(values, agg, std::move(result));
}
Expand Down
6 changes: 3 additions & 3 deletions cpp/src/interop/from_arrow.cu
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ struct dispatch_to_cudf_column {
stream,
mr);

col->set_null_mask(std::move(out_mask));
col->set_null_mask(std::move(out_mask), array.null_count());
}

return col;
Expand Down Expand Up @@ -220,7 +220,7 @@ std::unique_ptr<column> dispatch_to_cudf_column::operator()<numeric::decimal128>
return rmm::device_buffer{};
}();

col->set_null_mask(std::move(null_mask));
col->set_null_mask(std::move(null_mask), array.null_count());
return col;
}

Expand Down Expand Up @@ -254,7 +254,7 @@ std::unique_ptr<column> dispatch_to_cudf_column::operator()<bool>(
stream,
mr);

out_col->set_null_mask(std::move(out_mask));
out_col->set_null_mask(std::move(out_mask), array.null_count());
}

return out_col;
Expand Down
7 changes: 4 additions & 3 deletions cpp/src/structs/utilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ struct table_flattener {
validity_as_column.push_back(cudf::detail::is_valid(col, stream, mr));
if (col.has_nulls()) {
// copy bitmask is needed only if the column has null
validity_as_column.back()->set_null_mask(cudf::detail::copy_bitmask(col, stream, mr));
validity_as_column.back()->set_null_mask(cudf::detail::copy_bitmask(col, stream, mr),
col.null_count());
}
flat_columns.push_back(validity_as_column.back()->view());
if (not column_order.empty()) { flat_column_order.push_back(col_order); } // doesn't matter.
Expand Down Expand Up @@ -244,8 +245,8 @@ std::unique_ptr<column> superimpose_nulls_no_sanitize(bitmask_type const* null_m
auto const num_rows = input->size();

if (!input->nullable()) {
input->set_null_mask(cudf::detail::copy_bitmask(null_mask, 0, num_rows, stream, mr));
input->set_null_count(null_count);
input->set_null_mask(cudf::detail::copy_bitmask(null_mask, 0, num_rows, stream, mr),
null_count);
} else {
auto current_mask = input->mutable_view().null_mask();
std::vector<bitmask_type const*> masks{reinterpret_cast<bitmask_type const*>(null_mask),
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/unary/cast_ops.cu
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ std::unique_ptr<column> rescale(column_view input,
auto output_column = make_column_from_scalar(*scalar, input.size(), stream, mr);
if (input.nullable()) {
auto const null_mask = copy_bitmask(input, stream, mr);
output_column->set_null_mask(std::move(null_mask));
output_column->set_null_mask(std::move(null_mask), input.null_count());
}
return output_column;
}
Expand Down

0 comments on commit 9d36716

Please sign in to comment.