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.

Sign up for GitHub

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

Enable compiled binary ops in libcudf, python and java #8741

Merged
merged 17 commits into from
Aug 23, 2021
Merged

Enable compiled binary ops in libcudf, python and java #8741

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1ccc242
add experimental::detail::binary_operation
karthikeyann Jul 14, 2021
0734a85
use experimental::detail::binary_operation in libcudf functions
karthikeyann Jul 14, 2021
d65fe3a
use compiled experimental::binary_operation in python
karthikeyann Jul 14, 2021
010bd4a
use compiled experimental::binary_operation in java
karthikeyann Jul 14, 2021
0f35ff2
add logical_and logical_or to bool output type list
karthikeyann Jul 14, 2021
67c94ab
add NullMax, NullMin string unit tests
karthikeyann Jul 15, 2021
06fb13a
Merge branch 'branch-21.08' of github.com:rapidsai/cudf into fea-enab…
karthikeyann Jul 20, 2021
6684fe2
enable decimal32 decimal64 conversions
karthikeyann Jul 20, 2021
dd9fc3e
fix merge error
karthikeyann Jul 20, 2021
524bdcf
fixed point scalar to device_view fix mem access
karthikeyann Jul 22, 2021
cd3c244
Merge branch 'branch-21.10' of github.com:rapidsai/cudf into fea-enab…
karthikeyann Aug 1, 2021
50e4437
remove unused include
karthikeyann Aug 2, 2021
0f5d3cf
reorg code, jit::binary_operation, compiled as cudf::binary_operation
karthikeyann Aug 16, 2021
35c9ebb
review comments jni, cython fix as well
karthikeyann Aug 16, 2021
61e22bd
remove debug rmm_mode=cuda cmake change
karthikeyann Aug 17, 2021
62b52d7
delete commented code
karthikeyann Aug 19, 2021
e7e81ce
Merge branch 'branch-21.10' of github.com:rapidsai/cudf into fea-enab…
karthikeyann Aug 23, 2021
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
46 changes: 46 additions & 0 deletions cpp/include/cudf/detail/binaryop.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,50 @@ std::unique_ptr<column> binary_operation(
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());

} // namespace detail

namespace experimental {
namespace detail {
/**
* @copydoc cudf::experimental::binary_operation(scalar const&, column_view const&, binary_operator,
* data_type, rmm::mr::device_memory_resource *)
*
* @param stream CUDA stream used for device memory operations and kernel launches.
Copy link
Contributor

Choose a reason for hiding this comment

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

Parameter order here is incorrect. Should be stream, mr

Same applies for the other 3 declarations in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@copydoc is used at many places in our cpp documentation to avoid duplication of doc text, most often used in detail APIs. There is no cleaner way so far.

Note: Alternatively, we could contribute to doxygen to reorder the @param as listed in the function signature. Timeline: Uncertain.

Copy link
Contributor

Choose a reason for hiding this comment

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

@copydoc is used at many places in our cpp documentation to avoid duplication of doc text, most often used in detail APIs. There is no cleaner way so far.

Right, but isn't this going to result in incorrect documentation? My expectation is that you will end up getting something like:

 * @param lhs         The left operand scalar
 * @param rhs         The right operand column
 * @param op          The binary operator
 * @param output_type The desired data type of the output column
 * @param mr          Device memory resource used to allocate the returned column's device memory
 * @param stream    stream CUDA stream used for device memory operations and kernel launches.
 * @return            Output column of `output_type` type containing the result of
 *                    the binary operation
 * @throw cudf::logic_error if @p output_type dtype isn't fixed-width

...which is the incorrect order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how it's rendered now.
image

Copy link
Contributor Author

@karthikeyann karthikeyann Jul 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This is not better too.
(I generated this temporarily to show. it's not in released documentation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

detail APIs are not in released libcudf documentation. They are excluded by doxygen.

*/
std::unique_ptr<column> binary_operation(
scalar const& lhs,
column_view const& rhs,
binary_operator op,
data_type output_type,
rmm::cuda_stream_view stream = rmm::cuda_stream_default,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());

/**
* @copydoc cudf::experimental::binary_operation(column_view const&, scalar const&, binary_operator,
* data_type, rmm::mr::device_memory_resource *)
*
* @param stream CUDA stream used for device memory operations and kernel launches.
*/
std::unique_ptr<column> binary_operation(
column_view const& lhs,
scalar const& rhs,
binary_operator op,
data_type output_type,
rmm::cuda_stream_view stream = rmm::cuda_stream_default,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());

/**
* @copydoc cudf::experimental::binary_operation(column_view const&, column_view const&,
* binary_operator, data_type, rmm::mr::device_memory_resource *)
*
* @param stream CUDA stream used for device memory operations and kernel launches.
*/
std::unique_ptr<column> binary_operation(
column_view const& lhs,
column_view const& rhs,
binary_operator op,
data_type output_type,
rmm::cuda_stream_view stream = rmm::cuda_stream_default,
rmm::mr::device_memory_resource* mr = rmm::mr::get_current_device_resource());
} // namespace detail
} // namespace experimental
} // namespace cudf
59 changes: 46 additions & 13 deletions cpp/src/binaryop/binaryop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ std::unique_ptr<column> binary_operation(scalar const& lhs,
rmm::mr::device_memory_resource* mr)
{
if (lhs.type().id() == type_id::STRING and rhs.type().id() == type_id::STRING)
return experimental::binary_operation(lhs, rhs, op, output_type, mr);
return experimental::detail::binary_operation(lhs, rhs, op, output_type, stream, mr);

if (is_fixed_point(lhs.type()) or is_fixed_point(rhs.type()))
return fixed_point_binary_operation(lhs, rhs, op, output_type, stream, mr);
Expand All @@ -615,7 +615,7 @@ std::unique_ptr<column> binary_operation(column_view const& lhs,
rmm::mr::device_memory_resource* mr)
{
if (lhs.type().id() == type_id::STRING and rhs.type().id() == type_id::STRING)
return experimental::binary_operation(lhs, rhs, op, output_type, mr);
return experimental::detail::binary_operation(lhs, rhs, op, output_type, stream, mr);

if (is_fixed_point(lhs.type()) or is_fixed_point(rhs.type()))
return fixed_point_binary_operation(lhs, rhs, op, output_type, stream, mr);
Expand Down Expand Up @@ -644,7 +644,7 @@ std::unique_ptr<column> binary_operation(column_view const& lhs,
CUDF_EXPECTS(lhs.size() == rhs.size(), "Column sizes don't match");

if (lhs.type().id() == type_id::STRING and rhs.type().id() == type_id::STRING)
return experimental::binary_operation(lhs, rhs, op, output_type, mr);
return experimental::detail::binary_operation(lhs, rhs, op, output_type, stream, mr);

if (is_fixed_point(lhs.type()) or is_fixed_point(rhs.type()))
return fixed_point_binary_operation(lhs, rhs, op, output_type, stream, mr);
Expand Down Expand Up @@ -759,6 +759,7 @@ std::unique_ptr<column> binary_operation(column_view const& lhs,

// Experimental Compiled Binary operation
namespace experimental {
namespace binops {
namespace detail {
/**
* @copydoc cudf::experimental::binary_operation(column_view const&, column_view const&,
Expand All @@ -780,16 +781,11 @@ std::unique_ptr<column> binary_operation(LhsType const& lhs,
if (lhs.type().id() == type_id::STRING and rhs.type().id() == type_id::STRING and
output_type.id() == type_id::STRING and
(op == binary_operator::NULL_MAX or op == binary_operator::NULL_MIN))
return binops::compiled::string_null_min_max(lhs, rhs, op, output_type, stream, mr);
return cudf::binops::compiled::string_null_min_max(lhs, rhs, op, output_type, stream, mr);

if (not binops::compiled::is_supported_operation(output_type, lhs.type(), rhs.type(), op))
if (not cudf::binops::compiled::is_supported_operation(output_type, lhs.type(), rhs.type(), op))
CUDF_FAIL("Unsupported operator for these types");

// TODO check if scale conversion required?
// if (is_fixed_point(lhs.type()) or is_fixed_point(rhs.type()))
// CUDF_FAIL("Not yet supported fixed_point");
// return fixed_point_binary_operation(lhs, rhs, op, output_type, stream, mr);

auto out = make_fixed_width_column_for_output(lhs, rhs, op, output_type, stream, mr);

if constexpr (std::is_same_v<LhsType, column_view>)
Expand All @@ -802,6 +798,40 @@ std::unique_ptr<column> binary_operation(LhsType const& lhs,
return out;
}
} // namespace detail
} // namespace binops

namespace detail {
std::unique_ptr<column> binary_operation(scalar const& lhs,
column_view const& rhs,
binary_operator op,
data_type output_type,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
{
return cudf::experimental::binops::detail::binary_operation(
lhs, rhs, op, output_type, rmm::cuda_stream_default, mr);
}
std::unique_ptr<column> binary_operation(column_view const& lhs,
scalar const& rhs,
binary_operator op,
data_type output_type,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
{
return cudf::experimental::binops::detail::binary_operation(
lhs, rhs, op, output_type, rmm::cuda_stream_default, mr);
}
std::unique_ptr<column> binary_operation(column_view const& lhs,
column_view const& rhs,
binary_operator op,
data_type output_type,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource* mr)
{
return cudf::experimental::binops::detail::binary_operation(
lhs, rhs, op, output_type, rmm::cuda_stream_default, mr);
}
} // namespace detail

std::unique_ptr<column> binary_operation(scalar const& lhs,
column_view const& rhs,
Expand All @@ -810,7 +840,8 @@ std::unique_ptr<column> binary_operation(scalar const& lhs,
rmm::mr::device_memory_resource* mr)
{
CUDF_FUNC_RANGE();
return detail::binary_operation(lhs, rhs, op, output_type, rmm::cuda_stream_default, mr);
return experimental::detail::binary_operation(
lhs, rhs, op, output_type, rmm::cuda_stream_default, mr);
}
std::unique_ptr<column> binary_operation(column_view const& lhs,
scalar const& rhs,
Expand All @@ -819,7 +850,8 @@ std::unique_ptr<column> binary_operation(column_view const& lhs,
rmm::mr::device_memory_resource* mr)
{
CUDF_FUNC_RANGE();
return detail::binary_operation(lhs, rhs, op, output_type, rmm::cuda_stream_default, mr);
return experimental::detail::binary_operation(
lhs, rhs, op, output_type, rmm::cuda_stream_default, mr);
}
std::unique_ptr<column> binary_operation(column_view const& lhs,
column_view const& rhs,
Expand All @@ -828,7 +860,8 @@ std::unique_ptr<column> binary_operation(column_view const& lhs,
rmm::mr::device_memory_resource* mr)
{
CUDF_FUNC_RANGE();
return detail::binary_operation(lhs, rhs, op, output_type, rmm::cuda_stream_default, mr);
return experimental::detail::binary_operation(
lhs, rhs, op, output_type, rmm::cuda_stream_default, mr);
}
} // namespace experimental
} // namespace cudf
14 changes: 7 additions & 7 deletions cpp/src/groupby/hash/groupby.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,13 @@ class hash_compound_agg_finalizer final : public cudf::detail::aggregation_final
column_view sum_result = dense_results->get_result(col_idx, *sum_agg);
column_view count_result = dense_results->get_result(col_idx, *count_agg);

auto result =
cudf::detail::binary_operation(sum_result,
count_result,
binary_operator::DIV,
cudf::detail::target_type(result_type, aggregation::MEAN),
stream,
mr);
auto result = cudf::experimental::detail::binary_operation(
sum_result,
count_result,
binary_operator::DIV,
cudf::detail::target_type(result_type, aggregation::MEAN),
stream,
mr);
dense_results->add_result(col_idx, agg, std::move(result));
}

Expand Down
14 changes: 7 additions & 7 deletions cpp/src/groupby/sort/aggregate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,13 @@ void aggregate_result_functor::operator()<aggregation::MEAN>(aggregation const&

// TODO (dm): Special case for timestamp. Add target_type_impl for it.
// Blocked until we support operator+ on timestamps
auto result =
cudf::detail::binary_operation(sum_result,
count_result,
binary_operator::DIV,
cudf::detail::target_type(values.type(), aggregation::MEAN),
stream,
mr);
auto result = cudf::experimental::detail::binary_operation(
sum_result,
count_result,
binary_operator::DIV,
cudf::detail::target_type(values.type(), aggregation::MEAN),
stream,
mr);
cache.add_result(col_idx, agg, std::move(result));
};

Expand Down
6 changes: 4 additions & 2 deletions cpp/src/unary/cast_ops.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,14 @@ std::unique_ptr<column> rescale(column_view input,
if (input.type().scale() > scale) {
auto const scalar = make_fixed_point_scalar<T>(0, scale_type{scale});
auto const type = cudf::data_type{cudf::type_to_id<T>(), scale};
return detail::binary_operation(input, *scalar, binary_operator::ADD, type, stream, mr);
return experimental::detail::binary_operation(
input, *scalar, binary_operator::ADD, type, stream, mr);
} else {
auto const diff = input.type().scale() - scale;
auto const scalar = make_fixed_point_scalar<T>(std::pow(10, -diff), scale_type{diff});
auto const type = cudf::data_type{cudf::type_to_id<T>(), scale};
return detail::binary_operation(input, *scalar, binary_operator::DIV, type, stream, mr);
return experimental::detail::binary_operation(
input, *scalar, binary_operator::DIV, type, stream, mr);
}
};

Expand Down
8 changes: 4 additions & 4 deletions java/src/main/native/src/ColumnViewJni.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1104,8 +1104,8 @@ JNIEXPORT jlong JNICALL Java_ai_rapids_cudf_ColumnView_binaryOpVV(JNIEnv *env, j

cudf::data_type n_data_type = cudf::jni::make_data_type(out_dtype, scale);
cudf::binary_operator op = static_cast<cudf::binary_operator>(int_op);
std::unique_ptr<cudf::column> result = cudf::binary_operation(
*lhs, *rhs, op, n_data_type);
std::unique_ptr<cudf::column> result =
cudf::experimental::binary_operation(*lhs, *rhs, op, n_data_type);
return reinterpret_cast<jlong>(result.release());
}
CATCH_STD(env, 0);
Expand Down Expand Up @@ -1136,8 +1136,8 @@ JNIEXPORT jlong JNICALL Java_ai_rapids_cudf_ColumnView_binaryOpVS(JNIEnv *env, j
cudf::data_type n_data_type = cudf::jni::make_data_type(out_dtype, scale);

cudf::binary_operator op = static_cast<cudf::binary_operator>(int_op);
std::unique_ptr<cudf::column> result = cudf::binary_operation(
*lhs, *rhs, op, n_data_type);
std::unique_ptr<cudf::column> result =
cudf::experimental::binary_operation(*lhs, *rhs, op, n_data_type);
return reinterpret_cast<jlong>(result.release());
}
CATCH_STD(env, 0);
Expand Down
3 changes: 2 additions & 1 deletion java/src/main/native/src/ScalarJni.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,8 @@ JNIEXPORT jlong JNICALL Java_ai_rapids_cudf_Scalar_binaryOpSV(JNIEnv *env, jclas
cudf::data_type n_data_type = cudf::jni::make_data_type(out_dtype, scale);

cudf::binary_operator op = static_cast<cudf::binary_operator>(int_op);
std::unique_ptr<cudf::column> result = cudf::binary_operation(*lhs, *rhs, op, n_data_type);
std::unique_ptr<cudf::column> result =
cudf::experimental::binary_operation(*lhs, *rhs, op, n_data_type);
return reinterpret_cast<jlong>(result.release());
}
CATCH_STD(env, 0);
Expand Down
6 changes: 3 additions & 3 deletions python/cudf/cudf/_lib/binaryop.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ cdef binaryop_v_v(Column lhs, Column rhs,

with nogil:
c_result = move(
cpp_binaryop.binary_operation(
cpp_binaryop.compiled_binary_operation(
c_lhs,
c_rhs,
c_op,
Expand All @@ -124,7 +124,7 @@ cdef binaryop_v_s(Column lhs, DeviceScalar rhs,

with nogil:
c_result = move(
cpp_binaryop.binary_operation(
cpp_binaryop.compiled_binary_operation(
c_lhs,
c_rhs[0],
c_op,
Expand All @@ -143,7 +143,7 @@ cdef binaryop_s_v(DeviceScalar lhs, Column rhs,

with nogil:
c_result = move(
cpp_binaryop.binary_operation(
cpp_binaryop.compiled_binary_operation(
c_lhs[0],
c_rhs,
c_op,
Expand Down
24 changes: 24 additions & 0 deletions python/cudf/cudf/_lib/cpp/binaryop.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,27 @@ cdef extern from "cudf/binaryop.hpp" namespace "cudf" nogil:
const string& op,
data_type output_type
) except +

unique_ptr[column] compiled_binary_operation \
"cudf::experimental::binary_operation" (
const column_view& lhs,
const column_view& rhs,
binary_operator op,
data_type output_type
) except +

unique_ptr[column] compiled_binary_operation \
"cudf::experimental::binary_operation" (
const column_view& lhs,
const scalar& rhs,
binary_operator op,
data_type output_type
) except +

unique_ptr[column] compiled_binary_operation \
"cudf::experimental::binary_operation" (
const scalar& lhs,
const column_view& rhs,
binary_operator op,
data_type output_type
) except +