Add groupby product support

[karthikeyann]

closes #4882

Added groupby.product support in both hash and sort groupby.

[jrhemstad]

Doesn't target_type_impl need to be updated?

cudf/cpp/include/cudf/detail/aggregation/aggregation.hpp

Line 491 in ad9212b

struct target_type_impl<SourceType, aggregation::VARIANCE> {

[karthikeyann]

Doesn't target_type_impl need to be updated?

It's enabled with is_sum_product_agg.

cudf/cpp/include/cudf/detail/aggregation/aggregation.hpp

Line 460 in ad9212b

std::enable_if_t<std::is_integral<Source>::value && is_sum_product_agg(k)>> {

cudf/cpp/include/cudf/detail/aggregation/aggregation.hpp

Line 478 in ad9212b

std::enable_if_t<std::is_floating_point<Source>::value && is_sum_product_agg(k)>> {

[codecov]

Codecov Report

Merging #7763 (98b2218) into branch-0.20 (51336df) will increase coverage by 0.03%.
The diff coverage is 88.67%.

@@               Coverage Diff               @@
##           branch-0.20    #7763      +/-   ##
===============================================
+ Coverage        82.88%   82.92%   +0.03%     
===============================================
  Files              103      103              
  Lines            17668    17664       -4     
===============================================
+ Hits             14645    14648       +3     
+ Misses            3023     3016       -7     

Impacted Files	Coverage Δ
python/cudf/cudf/core/column/__init__.py	100.00% <ø> (ø)
python/cudf/cudf/utils/cudautils.py	57.75% <25.00%> (ø)
python/cudf/cudf/core/column/column.py	88.64% <71.42%> (ø)
python/cudf/cudf/core/column/numerical.py	94.43% <72.72%> (ø)
python/dask_cudf/dask_cudf/backends.py	89.51% <80.00%> (-0.08%)	⬇️
python/cudf/cudf/core/dataframe.py	90.87% <83.33%> (+0.01%)	⬆️
python/cudf/cudf/utils/utils.py	89.53% <91.66%> (+0.02%)	⬆️
python/cudf/cudf/core/column/datetime.py	89.91% <100.00%> (ø)
python/cudf/cudf/core/column/timedelta.py	88.66% <100.00%> (ø)
python/cudf/cudf/core/groupby/groupby.py	92.33% <100.00%> (+0.88%)	⬆️
... and 16 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c2f744...98b2218. Read the comment docs.

[karthikeyann]

rerun tests

[vyasr]

In addition to the inline comments, I had one additional question that isn't relevant to this exact PR but I'd appreciate an answer on (I don't know this part of the C++ code base very well yet): What is the purpose of the elementwise_aggregator? It seems like an unnecessary level of indirection between aggregate_row and update_target_element since the same template specializations could be applied without it.

[vyasr]

Suggested change

	// This test will not work until the following ptxas bug is fixed in 10.2
	// https://nvbugswb.nvidia.com/NvBugs5/SWBug.aspx?bugid=3186317&cp=
	TYPED_TEST(groupby_product_test, DISABLED_dictionary)
	TYPED_TEST(groupby_product_test, dictionary)

[karthikeyann]

This may need another cleanup. At current state, code needs specialization for every keys type.
Also another PR #7949 is also adding few operations.

[vyasr]

OK, if you think it's best to wait to enable this test feel free to resolve.

[jrhemstad]

What is the purpose of the elementwise_aggregator? It seems like an unnecessary level of indirection between aggregate_row and update_target_element since the same template specializations could be applied without it.

elementwise_aggregator is the thing that is dispatched by dispatch_type_and_aggregation and presents the operator() template with the appropriate template type interface. You could put the "specializations" on that operator(), but that requires SFINAE and enable_if. By calling using the update_target_element type we can just use normal partial specializations of a type instead of SFINAE.

It also allows us to have the base case automatically handle any error paths that don't have specializations:

cudf/cpp/include/cudf/detail/aggregation/aggregation.cuh

Lines 95 to 108 in b8baed5

	template <typename Source,
	aggregation::Kind k,
	bool target_has_nulls,
	bool source_has_nulls,
	typename Enable = void>
	struct update_target_element {
	__device__ void operator()(mutable_column_device_view target,
	size_type target_index,
	column_device_view source,
	size_type source_index) const noexcept
	{
	cudf_assert(false and "Invalid source type and aggregation combination.");
	}
	};

[vyasr]

Looks good assuming no further action items on my one open thread.

[karthikeyann]

@gpucibot merge

[jrhemstad]

@gpucibot merge