Add support for unary negation operator #17560
Conversation
Matt711
added
feature request
github-actions
bot
added
libcudf
|
Nice! If @bdice is OK with this approach could you also update cudf Python as well? cudf/python/cudf/cudf/core/frame.py Line 1612 in ebad043 |
Ah yeah, thanks. Will do |
Exactly — thanks @mroeschke. The negation behavior for bools and non-numerical types is the thing I want to watch carefully, to be sure we match the appropriate conventions across all the APIs we serve. |
| template <typename T, CUDF_ENABLE_IF(!std::is_signed_v<T> && !cudf::is_duration_t<T>::value)> | ||
| T __device__ operator()(T data) | ||
| { | ||
| return data; | ||
| } |
There was a problem hiding this comment.
I am confused by this implementation. I thought we didn't want to provide an implementation for unsigned data types?
There was a problem hiding this comment.
I think that needs to happen at the dispatch layer, not SFINAE on the device operator itself. For example, we're not constraining types in other operators:
struct DeviceSqrt {
template <typename T>
__device__ T operator()(T data)
{
return std::sqrt(data);
}
};We only use SFINAE in this file when the implementation differs by type -- not to allow/disallow types.
Can we remove the SFINAE and just have this?
struct DeviceNegate {
template <typename T>
T __device__ operator()(T data)
{
return -data;
}
};Also related to https://github.com/rapidsai/cudf/pull/17560/files#r1878099060
There was a problem hiding this comment.
Let me post the compile error I got when I removed it.
| auto output = cudf::unary_operation(input, cudf::unary_operator::NEGATE); | ||
| CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected, output->view()); | ||
| } | ||
|
|
There was a problem hiding this comment.
We need another test that unsigned types, timestamps, strings, and other complex types fail.
| @@ -226,6 +228,17 @@ def as_numerical_column( | |||
| ) -> "cudf.core.column.NumericalColumn": | |||
| return unary.cast(self, dtype) # type: ignore[return-value] | |||
|
|
|||
| def unary_operator(self, unaryop: str) -> ColumnBase: | |||
There was a problem hiding this comment.
How did this work before? Could we use abs on a decimal column?
Ideally we should not have to hardcode the supported operators for each type in Python -- I'd rather catch an exception from C++ and re-raise it with a Python-friendly message.
Description
Closes #17538. This PR adds support for unary negation operator in libcudf and plumbs the changes through cudf python and cudf polars.
Checklist