Repository navigation
Support non-unique keys for dictionary column types #22839
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
Changes from 23 commits
adb5e88
b681491
f525d92
3c7bb7e
20a011b
14c1296
c5ab27b
9bce5c6
1217031
1c7f8cd
ed286ad
ff49295
b5c0f64
277840b
2d4373d
64d5acd
b9252c9
b79cbbe
08cc3cc
9b1c2c2
010a95b
df0882f
c379a9f
275c043
6489638
549df2f
27c1f2a
9cb3d3b
c38de88
0d182cb
e71a5bb
e9d45e5
b83ece0
b604531
f16be6d
410e759
de5e51b
d5a7508
60d17d3
d165781
b686565
cfd4dfa
9e952d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,6 +117,9 @@ std::unique_ptr<column> remove_unused_keys( | |
| * The indices are updated to reflect the position values of the new keys. | ||
| * Any indices pointing to removed keys sets those rows to null. | ||
| * | ||
| * Although duplicate keys are allowed, indices in the returned dictionary may | ||
| * only reference one of the duplicates. | ||
|
Comment on lines
+124
to
+125
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I read this 3 times before I started to understand it. What is guaranteed/preserved here? I think the number of keys in the output dictionary is It is hard to provide determinism and lossless conversion/transformation with the ambiguities introduced by duplicate keys. Maybe there's a better way to say this, or maybe the current state is fine.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We introduced non-determinism by no longer requiring sorted keys. |
||
| * | ||
| * @code{.pseudo} | ||
| * d1 = {keys=["a", "b", "c"], indices=[2, 0, 1, 2, 1]} | ||
| * d2 = set_keys(existing_dict, ["b","c","d"]) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,16 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION. | ||
| * SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| #include <cudf/column/column_device_view.cuh> | ||
| #include <cudf/column/column_view.hpp> | ||
| #include <cudf/detail/copy.hpp> | ||
| #include <cudf/detail/device_scalar.hpp> | ||
| #include <cudf/detail/iterator.cuh> | ||
| #include <cudf/detail/nvtx/ranges.hpp> | ||
| #include <cudf/detail/utilities/cuda.cuh> | ||
| #include <cudf/detail/utilities/device_operators.cuh> | ||
| #include <cudf/dictionary/detail/iterator.cuh> | ||
| #include <cudf/dictionary/dictionary_column_view.hpp> | ||
| #include <cudf/reduction.hpp> | ||
| #include <cudf/reduction/detail/reduction_functions.hpp> | ||
|
|
@@ -129,6 +129,94 @@ struct create_minmax_with_nulls { | |
| } | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Functor to copy a minmax_pair result to individual scalar instances. | ||
| * | ||
| * @tparam T type of the data | ||
| * @tparam ResultType result type to assign min, max to minmax_pair<T> | ||
| */ | ||
| template <typename T, typename ResultType = minmax_pair<T>> | ||
| struct assign_min_max { | ||
| __device__ void operator()() | ||
| { | ||
| *min_data = result->min_val; | ||
| *max_data = result->max_val; | ||
| } | ||
|
|
||
| ResultType* result; | ||
| T* min_data; | ||
| T* max_data; | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Computes a minmax_pair<T> reduction directly over a dictionary column's decoded key | ||
| * values, i.e. `keys[indices[i]]` for each row `i`. | ||
| * | ||
| * @tparam T The dictionary's key type | ||
| */ | ||
| template <typename T> | ||
| auto reduce_dictionary(column_view const& col, rmm::cuda_stream_view stream) | ||
| { | ||
| auto d_dictionary = column_device_view::create(col, stream); | ||
| if (col.has_nulls()) { | ||
| auto pair_to_minmax = thrust::make_transform_iterator( | ||
|
davidwendt marked this conversation as resolved.
Outdated
|
||
| cudf::dictionary::detail::make_dictionary_pair_iterator<T>(*d_dictionary, true), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Follow-up PR: Perhaps we can we add a benchmark, if none exists, to measure whether loading
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, some exploration would be good here. This could first do a groupby-count to determine which keys are present (possibly including null values) and then do the minmax reduction over the keys with nonzero counts. |
||
| create_minmax_with_nulls<T>{}); | ||
| return reduce_device(pair_to_minmax, col.size(), minmax_binary_op<T>{}, stream); | ||
| } else { | ||
| auto col_to_minmax = thrust::make_transform_iterator( | ||
|
davidwendt marked this conversation as resolved.
Outdated
|
||
| cudf::dictionary::detail::make_dictionary_iterator<T>(*d_dictionary), create_minmax<T>{}); | ||
| return reduce_device(col_to_minmax, col.size(), minmax_binary_op<T>{}, stream); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @brief Dispatch functor for minmax operation on a dictionary column, dispatched on the | ||
| * dictionary's key type. | ||
| */ | ||
| struct minmax_dictionary_functor { | ||
| template <typename T> | ||
| static constexpr bool is_supported() | ||
| { | ||
| return !cudf::is_dictionary<T>() && !std::is_same_v<T, cudf::list_view> && | ||
| !std::is_same_v<T, cudf::struct_view>; | ||
| } | ||
|
|
||
| template <typename T> | ||
| std::pair<std::unique_ptr<scalar>, std::unique_ptr<scalar>> operator()( | ||
| column_view const& col, rmm::cuda_stream_view stream, rmm::device_async_resource_ref mr) | ||
| requires(is_supported<T>() and !std::is_same_v<T, cudf::string_view>) | ||
| { | ||
| using storage_type = device_storage_type_t<T>; | ||
| auto dev_result = reduce_dictionary<storage_type>(col, stream); | ||
| using ScalarType = cudf::scalar_type_t<T>; | ||
| auto minimum = std::make_unique<ScalarType>(T{}, true, stream, mr); | ||
| auto maximum = std::make_unique<ScalarType>(T{}, true, stream, mr); | ||
| cudf::detail::device_single_thread( | ||
| assign_min_max<storage_type>{dev_result.data(), minimum->data(), maximum->data()}, stream); | ||
| return {std::move(minimum), std::move(maximum)}; | ||
| } | ||
|
davidwendt marked this conversation as resolved.
Outdated
|
||
|
|
||
| template <typename T> | ||
| std::pair<std::unique_ptr<scalar>, std::unique_ptr<scalar>> operator()( | ||
| column_view const& col, rmm::cuda_stream_view stream, rmm::device_async_resource_ref mr) | ||
| requires(std::is_same_v<T, cudf::string_view>) | ||
| { | ||
| auto dev_result = reduce_dictionary<cudf::string_view>(col, stream); | ||
| auto const host_result = dev_result.value(stream); | ||
| return {std::make_unique<string_scalar>(host_result.min_val, true, stream, mr), | ||
| std::make_unique<string_scalar>(host_result.max_val, true, stream, mr)}; | ||
| } | ||
|
|
||
| template <typename T> | ||
| std::pair<std::unique_ptr<scalar>, std::unique_ptr<scalar>> operator()( | ||
| column_view const&, rmm::cuda_stream_view, rmm::device_async_resource_ref) | ||
| requires(!is_supported<T>()) | ||
| { | ||
| CUDF_FAIL("dictionary key type not supported for minmax() operation"); | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Dispatch functor for minmax operation. | ||
| * | ||
|
|
@@ -160,25 +248,6 @@ struct minmax_functor { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * @brief Functor to copy a minmax_pair result to individual scalar instances. | ||
| * | ||
| * @tparam T type of the data | ||
| * @tparam ResultType result type to assign min, max to minmax_pair<T> | ||
| */ | ||
| template <typename T, typename ResultType = minmax_pair<T>> | ||
| struct assign_min_max { | ||
| __device__ void operator()() | ||
| { | ||
| *min_data = result->min_val; | ||
| *max_data = result->max_val; | ||
| } | ||
|
|
||
| ResultType* result; | ||
| T* min_data; | ||
| T* max_data; | ||
| }; | ||
|
|
||
| template <typename T> | ||
| std::pair<std::unique_ptr<scalar>, std::unique_ptr<scalar>> operator()( | ||
| cudf::column_view const& col, rmm::cuda_stream_view stream, rmm::device_async_resource_ref mr) | ||
|
|
@@ -223,15 +292,8 @@ struct minmax_functor { | |
| cudf::column_view const& col, rmm::cuda_stream_view stream, rmm::device_async_resource_ref mr) | ||
| requires(cudf::is_dictionary<T>()) | ||
| { | ||
| // computes minimum and maximum on the dictionary indices as dictionary32 values | ||
| auto d_indices = reduce<T>(col, stream); | ||
| auto const indices = d_indices.value(stream); | ||
| // use these values to slice the keys column (add 1 for complete inclusion) | ||
| auto keys = cudf::detail::slice(dictionary_column_view(col).keys(), | ||
| {indices.min_val.value(), indices.max_val.value() + 1}, | ||
| stream) | ||
| .front(); | ||
| return type_dispatcher(keys.type(), minmax_functor{}, keys, stream, mr); | ||
| auto const keys_type = dictionary_column_view(col).keys().type(); | ||
|
davidwendt marked this conversation as resolved.
|
||
| return type_dispatcher(keys_type, minmax_dictionary_functor{}, col, stream, mr); | ||
| } | ||
|
|
||
| template <typename T> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.