Skip to content
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.

Sign up for GitHub

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

Followup for null count fixup in row_conversion.cu. #1155

Merged
merged 4 commits into from
May 23, 2023
Merged

Followup for null count fixup in row_conversion.cu. #1155

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
21347a8
Followup for null count fixup in row_conversion.cu.
mythrocks May 22, 2023
ac31cce
Removed redundant null-count calculation.
mythrocks May 22, 2023
34d22b8
Message clarifying that null-count isn't already set.
mythrocks May 22, 2023
aae5120
Document why null masks are being set.
mythrocks May 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/main/cpp/src/row_conversion.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2259,8 +2259,15 @@ std::unique_ptr<table> convert_from_rows(lists_column_view const &input,
for (int i = 0; i < static_cast<int>(schema.size()); ++i) {
if (schema[i].id() == type_id::STRING) {
// stuff real string column
auto const null_count = string_row_offset_columns[string_idx]->null_count();
auto string_data = string_row_offset_columns[string_idx].release()->release();
auto const null_count = [&] {
// Null-count not set previously. Calculate, on the fly.
auto const &null_mask = *string_data.null_mask;
return null_mask.data() ?
cudf::null_count(static_cast<bitmask_type const *>(null_mask.data()), 0,
mythrocks marked this conversation as resolved.
Show resolved Hide resolved
num_rows) :
0;
}();
output_columns[i] =
make_strings_column(num_rows, std::move(string_col_offsets[string_idx]),
std::move(string_data_cols[string_idx]),
Expand Down Expand Up @@ -2325,6 +2332,12 @@ std::unique_ptr<table> convert_from_rows_fixed_width_optimized(
num_rows, num_columns, size_per_row, dev_column_start.data(), dev_column_size.data(),
dev_output_data.data(), dev_output_nm.data(), child.data<int8_t>());

// Set null counts, because output_columns are modified via mutable-view,
// in the kernel above.
// TODO(future): Consider setting null count in the kernel itself.
for (auto &col : output_columns) {
col->set_null_count(cudf::null_count(col->view().null_mask(), 0, col->size()));
}
mythrocks marked this conversation as resolved.
Show resolved Hide resolved
return std::make_unique<table>(std::move(output_columns));
} else {
CUDF_FAIL("Only fixed width types are currently supported");
Expand Down