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.

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

Add struct utility functions. #10776

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ add_library(
src/ast/expression_parser.cpp
src/ast/expressions.cpp
src/binaryop/binaryop.cpp
src/binaryop/compiled/binary_ops.cu
src/binaryop/compiled/Add.cu
src/binaryop/compiled/ATan2.cu
src/binaryop/compiled/BitwiseAnd.cu
Expand Down Expand Up @@ -220,6 +219,7 @@ add_library(
src/binaryop/compiled/ShiftRightUnsigned.cu
src/binaryop/compiled/Sub.cu
src/binaryop/compiled/TrueDiv.cu
src/binaryop/compiled/binary_ops.cu
src/binaryop/compiled/util.cpp
src/labeling/label_bins.cu
src/bitmask/null_mask.cu
Expand Down
16 changes: 15 additions & 1 deletion cpp/include/cudf/detail/structs/utilities.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2021, NVIDIA CORPORATION.
* Copyright (c) 2020-2022, 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 @@ -245,6 +245,20 @@ std::tuple<cudf::table_view, std::vector<rmm::device_buffer>> superimpose_parent
table_view const& table,
rmm::cuda_stream_view stream = rmm::cuda_stream_default,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());

/**
* @brief Checks if a column or any of its children is a struct column with structs that are null.
*
* This function searches for structs that are null -- differentiating between structs that are null
* and structs containing null values. Null structs add a column to the result of the flatten column
* utility and necessitates column_nullability::FORCE when flattening the column for comparison
Comment on lines +253 to +254
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi, given that this is for the legacy flattening, I suspect the need for this function will go away with using the new comparators. It's fine to merge for now, but just keep this in mind.

* operations.
*
* @param col Column to check for null structs
* @return A boolean indicating if the column is or contains a struct column that contains a null
* struct.
*/
bool contains_null_structs(column_view const& col);
} // namespace detail
} // namespace structs
} // namespace cudf
2 changes: 1 addition & 1 deletion cpp/include/cudf/table/row_operators.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ __device__ weak_ordering compare_elements(Element lhs, Element rhs)
* @brief A specialization for floating-point `Element` type relational comparison
* to derive the order of the elements with respect to `lhs`.
*
* This Specialization handles `nan` in the following order:
* This specialization handles `nan` in the following order:
* `[-Inf, -ve, 0, -0, +ve, +Inf, NaN, NaN, null] (for null_order::AFTER)`
* `[null, -Inf, -ve, 0, -0, +ve, +Inf, NaN, NaN] (for null_order::BEFORE)`
*
Expand Down
31 changes: 31 additions & 0 deletions cpp/include/cudf/utilities/traits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,37 @@ constexpr inline bool is_nested(data_type type)
return cudf::type_dispatcher(type, is_nested_impl{});
}

/**
* @brief Indicates whether `T` is a struct type.
*
* @param T The type to verify
* @return A boolean indicating if T is a struct type
*/
template <typename T>
constexpr inline bool is_struct()
{
return std::is_same_v<T, cudf::struct_view>;
}

struct is_struct_impl {
template <typename T>
constexpr bool operator()()
{
return is_struct<T>();
}
};

/**
* @brief Indicates whether `type` is a struct type.
*
* @param type The `data_type` to verify
* @return A boolean indicating if `type` is a struct type
*/
constexpr inline bool is_struct(data_type type)
{
return cudf::type_dispatcher(type, is_struct_impl{});
}

template <typename FromType>
struct is_bit_castable_to_impl {
template <typename ToType, std::enable_if_t<is_compound<ToType>()>* = nullptr>
Expand Down
6 changes: 6 additions & 0 deletions cpp/src/structs/utilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,12 @@ std::tuple<cudf::table_view, std::vector<rmm::device_buffer>> superimpose_parent
return {table_view{superimposed_columns}, std::move(superimposed_nullmasks)};
}

bool contains_null_structs(column_view const& col)
{
return (is_struct(col) && col.has_nulls()) ||
std::any_of(col.child_begin(), col.child_end(), contains_null_structs);
}

Comment on lines +444 to +449
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is very short, I wonder why don't we inline it in the header?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is very short, I wonder why don't we inline it in the header?

I don't agree with this assessment for moving a function to a header. Moving it to a header may require additional includes in the header that are unnecessary to the caller. I think we should only inline host functions in headers when absolutely necessary. It can improve compile time and also perhaps encapsulation such that changing the implementation (as in this case) would not require recompiling all the source files that include the header.

} // namespace detail
} // namespace structs
} // namespace cudf