diff --git a/cpp/include/cudf/detail/aggregation/aggregation.cuh b/cpp/include/cudf/detail/aggregation/aggregation.cuh index aece4d53faf..f7ed66cf386 100644 --- a/cpp/include/cudf/detail/aggregation/aggregation.cuh +++ b/cpp/include/cudf/detail/aggregation/aggregation.cuh @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2020, NVIDIA CORPORATION. + * Copyright (c) 2019-2021, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -432,6 +432,8 @@ struct identity_initializer { template static constexpr bool is_supported() { + // Note: !is_fixed_point() means that aggregations for fixed_point should happen on the + // underlying type (see device_storage_type_t), not that fixed_point is not supported return cudf::is_fixed_width() && !is_fixed_point() and (k == aggregation::SUM or k == aggregation::MIN or k == aggregation::MAX or k == aggregation::COUNT_VALID or k == aggregation::COUNT_ALL or diff --git a/cpp/include/cudf/utilities/type_dispatcher.hpp b/cpp/include/cudf/utilities/type_dispatcher.hpp index 9045c596b62..271d722396e 100644 --- a/cpp/include/cudf/utilities/type_dispatcher.hpp +++ b/cpp/include/cudf/utilities/type_dispatcher.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2020, NVIDIA CORPORATION. + * Copyright (c) 2019-2021, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -102,17 +102,32 @@ using device_storage_type_t = std::conditional_t::value, int64_t, T>>; // clang-format on +/** + * @brief Returns the corresponding `type_id` of type stored on device for a given `type_id` + * + * @param id The given `type_id` + * @return Corresponding `type_id` of type stored on device + */ +inline type_id device_storage_type_id(type_id id) +{ + switch (id) { + case type_id::DECIMAL32: return type_id::INT32; + case type_id::DECIMAL64: return type_id::INT64; + default: return id; + } +} + /** * @brief Checks if `fixed_point`-like types have template type `T` matching the column's * stored type id * - * @tparam T The type that is stored on the device - * @param id The `data_type::id` of the column - * @return true If T matches the stored column type id - * @return false If T does not match the stored column type id + * @tparam T The type that is stored on the device + * @param id The `data_type::id` of the column + * @return `true` If T matches the stored column `type_id` + * @return `false` If T does not match the stored column `type_id` */ template -bool type_id_matches_device_storage_type(type_id const& id) +bool type_id_matches_device_storage_type(type_id id) { return (id == type_id::DECIMAL32 && std::is_same::value) || (id == type_id::DECIMAL64 && std::is_same::value) || id == type_to_id(); diff --git a/cpp/src/aggregation/aggregation.cu b/cpp/src/aggregation/aggregation.cu index 564713e959b..b9193345c94 100644 --- a/cpp/src/aggregation/aggregation.cu +++ b/cpp/src/aggregation/aggregation.cu @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, NVIDIA CORPORATION. + * Copyright (c) 2020-2021, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,8 +27,9 @@ void initialize_with_identity(mutable_table_view& table, // TODO: Initialize all the columns in a single kernel instead of invoking one // kernel per column for (size_type i = 0; i < table.num_columns(); ++i) { - auto col = table.column(i); - dispatch_type_and_aggregation(col.type(), aggs[i], identity_initializer{}, col, stream); + auto col = table.column(i); + auto const type = data_type{device_storage_type_id(col.type().id())}; + dispatch_type_and_aggregation(type, aggs[i], identity_initializer{}, col, stream); } } diff --git a/cpp/src/groupby/sort/group_single_pass_reduction_util.cuh b/cpp/src/groupby/sort/group_single_pass_reduction_util.cuh index 7fc80c14171..5b26b7bf108 100644 --- a/cpp/src/groupby/sort/group_single_pass_reduction_util.cuh +++ b/cpp/src/groupby/sort/group_single_pass_reduction_util.cuh @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2020, NVIDIA CORPORATION. + * Copyright (c) 2019-2021, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,7 +41,7 @@ struct reduce_functor { static constexpr bool is_supported() { if (K == aggregation::SUM) - return cudf::is_numeric() || cudf::is_duration(); + return cudf::is_numeric() || cudf::is_duration() || cudf::is_fixed_point(); else if (K == aggregation::MIN or K == aggregation::MAX) return cudf::is_fixed_width() and is_relationally_comparable(); else if (K == aggregation::ARGMIN or K == aggregation::ARGMAX) @@ -58,11 +58,16 @@ struct reduce_functor { rmm::cuda_stream_view stream, rmm::mr::device_memory_resource* mr) { + using DeviceType = device_storage_type_t; using OpType = cudf::detail::corresponding_operator_t; using ResultType = cudf::detail::target_type_t; + auto result_type = is_fixed_point() + ? data_type{type_to_id(), values.type().scale()} + : data_type{type_to_id()}; + std::unique_ptr result = - make_fixed_width_column(data_type(type_to_id()), + make_fixed_width_column(result_type, num_groups, values.has_nulls() ? mask_state::ALL_NULL : mask_state::UNALLOCATED, stream, @@ -83,7 +88,7 @@ struct reduce_functor { [d_values = *valuesview, d_result = *resultview, dest_indices = group_labels.data().get()] __device__(auto i) { - cudf::detail::update_target_element{}( + cudf::detail::update_target_element{}( d_result, dest_indices[i], d_values, i); }); } else { diff --git a/cpp/tests/groupby/group_count_test.cpp b/cpp/tests/groupby/group_count_test.cpp index 77d1e275d24..aca19a668e4 100644 --- a/cpp/tests/groupby/group_count_test.cpp +++ b/cpp/tests/groupby/group_count_test.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2020, NVIDIA CORPORATION. + * Copyright (c) 2019-2021, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -145,7 +145,6 @@ TYPED_TEST(groupby_count_test, null_keys_and_values) } - struct groupby_count_string_test : public cudf::test::BaseFixture {}; TEST_F(groupby_count_string_test, basic) @@ -168,6 +167,117 @@ TEST_F(groupby_count_string_test, basic) } // clang-format on +template +struct FixedPointTestBothReps : public cudf::test::BaseFixture { +}; + +TYPED_TEST_CASE(FixedPointTestBothReps, cudf::test::FixedPointTypes); + +TYPED_TEST(FixedPointTestBothReps, GroupByCount) +{ + using namespace numeric; + using decimalXX = TypeParam; + using RepType = cudf::device_storage_type_t; + using fp_wrapper = cudf::test::fixed_point_column_wrapper; + + using K = int32_t; + using V = decimalXX; + using R = cudf::detail::target_type_t; + + auto const scale = scale_type{-1}; + auto const keys = fixed_width_column_wrapper{1, 2, 3, 1, 2, 2, 1, 3, 3, 2}; + auto const vals = fp_wrapper{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, scale}; + + auto const expect_keys = fixed_width_column_wrapper{1, 2, 3}; + auto const expect_vals = fixed_width_column_wrapper{3, 4, 3}; + + auto agg = cudf::make_count_aggregation(); + test_single_agg(keys, vals, expect_keys, expect_vals, std::move(agg)); + + auto agg1 = cudf::make_count_aggregation(); + test_single_agg(keys, vals, expect_keys, expect_vals, std::move(agg1), force_use_sort_impl::YES); + + auto agg2 = cudf::make_count_aggregation(null_policy::INCLUDE); + test_single_agg(keys, vals, expect_keys, expect_vals, std::move(agg2)); +} + +TYPED_TEST(FixedPointTestBothReps, GroupBySumProductMinMaxDecimalAsValue) +{ + using namespace numeric; + using decimalXX = TypeParam; + using RepType = cudf::device_storage_type_t; + using fp_wrapper = cudf::test::fixed_point_column_wrapper; + using fp64_wrapper = cudf::test::fixed_point_column_wrapper; + using K = int32_t; + + for (auto const i : {2, 1, 0, -1, -2}) { + auto const scale = scale_type{i}; + auto const keys = fixed_width_column_wrapper{1, 2, 3, 1, 2, 2, 1, 3, 3, 2}; + auto const vals = fp_wrapper{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, scale}; + + auto const expect_keys = fixed_width_column_wrapper{1, 2, 3}; + + auto const expect_vals_sum = fp64_wrapper{{9, 19, 17}, scale}; + auto const expect_vals_min = fp_wrapper{{0, 1, 2}, scale}; + auto const expect_vals_max = fp_wrapper{{6, 9, 8}, scale}; + + auto agg1 = cudf::make_sum_aggregation(); + test_single_agg( + keys, vals, expect_keys, expect_vals_sum, std::move(agg1), force_use_sort_impl::YES); + + auto agg2 = cudf::make_min_aggregation(); + test_single_agg( + keys, vals, expect_keys, expect_vals_min, std::move(agg2), force_use_sort_impl::YES); + + auto agg3 = cudf::make_max_aggregation(); + test_single_agg( + keys, vals, expect_keys, expect_vals_max, std::move(agg3), force_use_sort_impl::YES); + + auto agg4 = cudf::make_product_aggregation(); + EXPECT_THROW( + test_single_agg(keys, vals, expect_keys, {}, std::move(agg4), force_use_sort_impl::YES), + cudf::logic_error); + } +} + +TYPED_TEST(FixedPointTestBothReps, GroupBySumProductMinMaxDecimalAsValueAndKey) +{ + using namespace numeric; + using decimalXX = TypeParam; + using RepType = cudf::device_storage_type_t; + using fp_wrapper = cudf::test::fixed_point_column_wrapper; + using fp64_wrapper = cudf::test::fixed_point_column_wrapper; + + for (auto const i : {2, 1, 0, -1, -2}) { + auto const scale = scale_type{i}; + auto const keys = fp_wrapper{{1, 2, 3, 1, 2, 2, 1, 3, 3, 2}, scale}; + auto const vals = fp_wrapper{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, scale}; + + auto const expect_keys = fp_wrapper{{1, 2, 3}, scale}; + + auto const expect_vals_sum = fp64_wrapper{{9, 19, 17}, scale}; + auto const expect_vals_min = fp_wrapper{{0, 1, 2}, scale}; + auto const expect_vals_max = fp_wrapper{{6, 9, 8}, scale}; + + auto agg1 = cudf::make_sum_aggregation(); + test_single_agg( + keys, vals, expect_keys, expect_vals_sum, std::move(agg1), force_use_sort_impl::YES); + + auto agg2 = cudf::make_min_aggregation(); + test_single_agg( + keys, vals, expect_keys, expect_vals_min, std::move(agg2), force_use_sort_impl::YES); + + auto agg3 = cudf::make_max_aggregation(); + test_single_agg( + keys, vals, expect_keys, expect_vals_max, std::move(agg3), force_use_sort_impl::YES); + + auto agg4 = cudf::make_product_aggregation(); + EXPECT_THROW( + test_single_agg(keys, vals, expect_keys, {}, std::move(agg4), force_use_sort_impl::YES), + cudf::logic_error); + } +} + struct groupby_dictionary_count_test : public cudf::test::BaseFixture { };