[REVIEW] libcudf: generic reduction and scan support

[kovaltan]

Merge gdf_sum, gdf_product, gdf_sum_of_squares, gdf_min, gdf_max into a single cudf::reduction API.
Reneme gdf_prefixsum into cudf::scan and support 'min', 'max', 'product' operation.
Add min and max to cudf::reduction for support non-arithmetic types, e.g., GDF_CATEGORY, GDF_DATE32, GDF_DATE64, GDF_TIMESTAMP.
Use single step reduction for cudf::reduction and remove gdf_reduce_optimal_output_size.
Use gdf_scalar for output of gdf_reduction.

Closes #443 (Add unit tests for functionality in reductions.cu)
Closes #446 (CUDA reductions should have independent input and output types)
Closes #954 (libcudf reductions should support non-arithmetic types for some operations)
Closes #978 (libcudf should provide a generic scan function)
Related #1224 (Calling sum on a boolean column returns a boolean)

reduction tasks:

• Add min, max operation support for non-arithmetic types (Issue [BUG] libcudf reductions should support non-arithmetic types for some operations #954)
• Separate DeviceOP structs to .cuh file which will be shared with scan
• Add gtest for non-arithmetic types (Issue Add unit tests for functionality in reductions.cu #443)
• Use single stage reduction using atomic operation
• Independent input and output types (Issue CUDA reductions should have independent input and output types #446)
• Use gdf_scalar for output of gdf_reduction
• Use GDF_EXPECTS macro and throw exceptions if failed.
• Remove gdf_reduce_optimal_output_size since it is required for 2-step reduction (related to Issue gdf_reduce_optimal_output_size() is a misnomer #610)
• merge APIs to a single generic gdf_reduction
• mention about overflow and output precision at API doc
• Add cython binding for gdf_reduction
• migrate python test cpp/python/libgdf_cffi/tests -> python/tests

scan tasks:

• Add min, max, product for scan
• Rename gdf_prefixsum into gdf_scan (Issue [FEA] libcudf should provide a generic scan function #978)
• Add gtest for gdf_scan
• Rename python binding for gdf_scan
• Update python test by using gdf_scan
• Use GDF_EXPECTS macro and throw exceptions if failed.
• migrate python test cpp/python/libgdf_cffi/tests -> python/tests
• add "dtype" paramter to python Series.min, max, sum. product, ...

This PL also included these macros: CUDF_FAIL CUDF_EXPECT_NO_THROW
Then, RMM_TRY has been changed to throw exception at failed.

Examples:

switch(enum e){
case Enum_1: ...
case Enum_2: ...
default:
  CUDF_FAIL("Invalid enum input");
}

CUDF_FAIL(msg) is same with CUDF_EXPECTS(false, msg)

CUDF_EXPECT_NO_THROW( gdf_scan(raw_input, raw_output, op, inclusive));

CUDF_EXPECT_NO_THROW is used only in gtests, and it is the utility macro for debugging.
The testing is same with EXPECT_NO_THROW in gtests, but it prints out the error message from CUDF_FAIL and CUDF_EXPECTS.

[felipeblazing]

Can we add some unit tests that perform aggregations for non-arithmetic types?

[jrhemstad]

See https://github.com/rapidsai/cudf/pull/892/files#diff-4b0b6cd3d7dabc1501cc00b5b13d9370R93 for gdf_scalar which should be used for the return value of a reduction.

[kovaltan]

Hi @jrhemstad
Regarding to the memory location of the result ofgdf_reduction.
If gdf_scalar is at host, gdf_scalar.data is also at host memory.

cudf/cpp/include/cudf/types.h

Lines 110 to 114 in 78a3cc8

	typedef struct {
	gdf_data data; /**< Pointer to the scalar data */
	gdf_dtype dtype; /**< The datatype of the scalar's data */
	bool is_valid; /**< False if the value is null */
	} gdf_scalar;

I'd like to make sure if you mean gdf_reduction should write back the result into host memory.
If the result should be at device memory, using pointer of gdf_data in device memory seems enough.

[eyalroz]

How does this new reduction code handle overflows?

[kovaltan]

Hi @eyalroz

How does this new reduction code handle overflows?

No, there is no plan to handle overflows.
Instead, this PL will support output type (Issue #446).

[GPUtester]

Can one of the admins verify this patch?

[eyalroz]

@kovaltan :

No, there is no plan to handle overflows.

So, I suggest that we should use the opportunity of touching this code to clearly document this fact in the relevant Doxygen comments.

[kovaltan]

So, I suggest that we should use the opportunity of touching this code to clearly document this fact in the relevant Doxygen comments.

OK, I will document it at API doc after I implement output precision support.

[kkraus14]

Print statements need to be removed and then question regarding overflows

Addressed