Skip to content

Commit

Permalink
Add tests of reflected ufuncs and fix behavior of logical reflected u…
Browse files Browse the repository at this point in the history
…funcs (#10261)

This PR fixes cases like `cupy.array([1, 2, 3]) < cudf.Series([1, 2, 3])`. The code would previously attempt to find a reflected operator `Series.__rle__`, but no such operator exists because of the inherent symmetry in comparison operators. These changes fix the detection of such operators to just use operator `__ge__` in this case.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Michael Wang (https://github.com/isVoid)
  - GALI PREM SAGAR (https://github.com/galipremsagar)

URL: #10261
  • Loading branch information
vyasr authored Feb 11, 2022
1 parent 8cc84c6 commit 4c92f86
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 10 deletions.
24 changes: 21 additions & 3 deletions python/cudf/cudf/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -998,9 +998,27 @@ def __array_ufunc__(self, ufunc, method, *inputs, **kwargs):
# First look for methods of the class.
fname = ufunc.__name__
if fname in binary_operations:
not_reflect = self is inputs[0]
other = inputs[not_reflect]
op = f"__{'' if not_reflect else 'r'}{binary_operations[fname]}__"
reflect = self is not inputs[0]
other = inputs[0] if reflect else inputs[1]

# These operators need to be mapped to their inverses when
# performing a reflected operation because no reflected version of
# the operators themselves exist.
ops_without_reflection = {
"gt": "lt",
"ge": "le",
"lt": "gt",
"le": "ge",
# ne and eq are symmetric, so they are their own inverse op
"ne": "ne",
"eq": "eq",
}

op = binary_operations[fname]
if reflect and op in ops_without_reflection:
op = ops_without_reflection[op]
reflect = False
op = f"__{'r' if reflect else ''}{op}__"

# pandas bitwise operations return bools if indexes are misaligned.
# TODO: Generalize for other types of Frames
Expand Down
35 changes: 28 additions & 7 deletions python/cudf/cudf/tests/test_array_ufunc.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Copyright (c) 2020-2022, NVIDIA CORPORATION.

import operator
from functools import reduce

Expand Down Expand Up @@ -98,17 +100,26 @@ def test_ufunc_series(ufunc, has_nulls, indexed):
raise


@pytest.mark.parametrize("ufunc", [np.add, np.greater, np.logical_and])
@pytest.mark.parametrize(
"ufunc", [np.add, np.greater, np.greater_equal, np.logical_and]
)
@pytest.mark.parametrize("has_nulls", [True, False])
@pytest.mark.parametrize("indexed", [True, False])
@pytest.mark.parametrize("type_", ["cupy", "numpy", "list"])
def test_binary_ufunc_series_array(ufunc, has_nulls, indexed, type_):
@pytest.mark.parametrize("reflect", [True, False])
def test_binary_ufunc_series_array(ufunc, has_nulls, indexed, type_, reflect):
fname = ufunc.__name__
if fname == "greater" and has_nulls:
if fname in ("greater", "greater_equal") and has_nulls:
pytest.xfail(
"The way cudf casts nans in arrays to nulls during binops with "
"cudf objects is currently incompatible with pandas."
)
if reflect and has_nulls and type_ == "cupy":
pytest.skip(
"When cupy is the left operand there is no way for us to avoid "
"calling its binary operators, which cannot handle cudf objects "
"that contain nulls."
)
N = 100
# Avoid zeros in either array to skip division by 0 errors. Also limit the
# scale to avoid issues with overflow, etc. We use ints because some
Expand Down Expand Up @@ -136,18 +147,28 @@ def test_binary_ufunc_series_array(ufunc, has_nulls, indexed, type_):
if type_ == "list":
arg1 = arg1.tolist()

got = ufunc(args[0], arg1)
expect = ufunc(args[0].to_pandas(), args[1].to_numpy())
if reflect:
got = ufunc(arg1, args[0])
expect = ufunc(args[1].to_numpy(), args[0].to_pandas())
else:
got = ufunc(args[0], arg1)
expect = ufunc(args[0].to_pandas(), args[1].to_numpy())

if ufunc.nout > 1:
for g, e in zip(got, expect):
if has_nulls:
e[mask] = np.nan
assert_eq(g, e)
if type_ == "cupy" and reflect:
assert (cp.asnumpy(g) == e).all()
else:
assert_eq(g, e)
else:
if has_nulls:
expect[mask] = np.nan
assert_eq(got, expect)
if type_ == "cupy" and reflect:
assert (cp.asnumpy(got) == expect).all()
else:
assert_eq(got, expect)


@pytest.mark.parametrize(
Expand Down

0 comments on commit 4c92f86

Please sign in to comment.