Skip to content

Commit c46a1c1

Browse files
Address review feedback on datetime masked ops (round 2)
Matt's review on NVIDIA#23309: - Inline the one-liner datetime test helpers/aliases (_DT/_TD, _dt_in/ _td_in, _dt_out/_td_out, _bool) at their use sites, matching the file's convention of writing types/arrays inline. - Document that _needs_datetimelike_delegate is a lowering-time gate downstream of typing: invalid temporal combinations (datetime+datetime, datetime<timedelta, ...) are already rejected during typing and never reach the delegate, so the permissive operand-type check is safe. Also note how a future unit-sensitive op (int*timedelta) would extend the gate.
1 parent aae47a4 commit c46a1c1

2 files changed

Lines changed: 64 additions & 54 deletions

File tree

‎python/cudf/cudf/core/udf/mlir_backend/masked_lowering.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,12 @@ def _needs_datetimelike_delegate(
213213
``1_000_000_000 < 2``, not ``1 < 2``).
214214
215215
So delegate for add/sub and all comparison operators when either operand is
216-
temporal.
216+
temporal. This is a lowering-time gate downstream of typing: invalid
217+
temporal combinations (``datetime + datetime``, ``datetime < timedelta``,
218+
...) are already rejected during typing, so the permissive operand-type
219+
check here never actually fires for them. Extending support to other
220+
unit-sensitive ops (e.g. ``int * timedelta``) would mean adding those
221+
operators to the set below.
217222
"""
218223
if op not in (operator.add, operator.sub, *comparison_ops):
219224
return False

‎python/cudf/cudf/tests/private_objects/mlir_backend/test_masked_lowering.py‎

Lines changed: 58 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -792,40 +792,17 @@ def k(out_v, out_valid, a, av):
792792
# allocated as int64 and viewed as the temporal dtype (the same trick
793793
# ``cudf.core.udf.utils._return_arr_from_dtype`` uses).
794794

795-
_DT = types.NPDatetime("ns")
796-
_TD = types.NPTimedelta("ns")
797-
798-
799-
def _dt_in(values):
800-
return cp.asarray(np.array(values, dtype="int64")).view("datetime64[ns]")
801-
802-
803-
def _td_in(values):
804-
return cp.asarray(np.array(values, dtype="int64")).view("timedelta64[ns]")
805-
806-
807-
def _dt_out():
808-
return cp.zeros(1, dtype=np.int64).view("datetime64[ns]")
809-
810-
811-
def _td_out():
812-
return cp.zeros(1, dtype=np.int64).view("timedelta64[ns]")
813-
814-
815-
def _bool(v):
816-
return cp.array([v], dtype=np.bool_)
817-
818795

819796
def test_masked_datetime_minus_datetime_is_timedelta():
820797
"""Masked ``datetime - datetime`` yields a Masked timedelta."""
821798

822799
@cuda.jit(
823800
types.void(
824-
_TD[::1],
801+
types.NPTimedelta("ns")[::1],
825802
types.boolean[::1],
826-
_DT[::1],
803+
types.NPDatetime("ns")[::1],
827804
types.boolean[::1],
828-
_DT[::1],
805+
types.NPDatetime("ns")[::1],
829806
types.boolean[::1],
830807
)
831808
)
@@ -834,16 +811,16 @@ def k(out_v, out_valid, a, av, b, bv):
834811
out_v[0] = m.value
835812
out_valid[0] = m.valid
836813

837-
out_v = _td_out()
814+
out_v = cp.zeros(1, dtype=np.int64).view("timedelta64[ns]")
838815
out_valid = cp.zeros(1, dtype=np.bool_)
839816
_launch(
840817
k,
841818
out_v,
842819
out_valid,
843-
_dt_in([1000]),
844-
_bool(True),
845-
_dt_in([400]),
846-
_bool(True),
820+
cp.asarray(np.array([1000], dtype="int64")).view("datetime64[ns]"),
821+
cp.array([True], dtype=np.bool_),
822+
cp.asarray(np.array([400], dtype="int64")).view("datetime64[ns]"),
823+
cp.array([True], dtype=np.bool_),
847824
)
848825
assert int(out_v.get().view("int64")[0]) == 600
849826
assert bool(out_valid.get()[0]) is True
@@ -854,18 +831,25 @@ def test_masked_datetime_plus_timedelta_is_datetime():
854831

855832
@cuda.jit(
856833
types.void(
857-
_DT[::1],
858-
_DT[::1],
834+
types.NPDatetime("ns")[::1],
835+
types.NPDatetime("ns")[::1],
859836
types.boolean[::1],
860-
_TD[::1],
837+
types.NPTimedelta("ns")[::1],
861838
types.boolean[::1],
862839
)
863840
)
864841
def k(out_v, a, av, t, tv):
865842
out_v[0] = (Masked(a[0], av[0]) + Masked(t[0], tv[0])).value
866843

867-
out_v = _dt_out()
868-
_launch(k, out_v, _dt_in([1000]), _bool(True), _td_in([250]), _bool(True))
844+
out_v = cp.zeros(1, dtype=np.int64).view("datetime64[ns]")
845+
_launch(
846+
k,
847+
out_v,
848+
cp.asarray(np.array([1000], dtype="int64")).view("datetime64[ns]"),
849+
cp.array([True], dtype=np.bool_),
850+
cp.asarray(np.array([250], dtype="int64")).view("timedelta64[ns]"),
851+
cp.array([True], dtype=np.bool_),
852+
)
869853
assert int(out_v.get().view("int64")[0]) == 1250
870854

871855

@@ -874,18 +858,25 @@ def test_masked_timedelta_plus_timedelta_is_timedelta():
874858

875859
@cuda.jit(
876860
types.void(
877-
_TD[::1],
878-
_TD[::1],
861+
types.NPTimedelta("ns")[::1],
862+
types.NPTimedelta("ns")[::1],
879863
types.boolean[::1],
880-
_TD[::1],
864+
types.NPTimedelta("ns")[::1],
881865
types.boolean[::1],
882866
)
883867
)
884868
def k(out_v, a, av, b, bv):
885869
out_v[0] = (Masked(a[0], av[0]) + Masked(b[0], bv[0])).value
886870

887-
out_v = _td_out()
888-
_launch(k, out_v, _td_in([300]), _bool(True), _td_in([120]), _bool(True))
871+
out_v = cp.zeros(1, dtype=np.int64).view("timedelta64[ns]")
872+
_launch(
873+
k,
874+
out_v,
875+
cp.asarray(np.array([300], dtype="int64")).view("timedelta64[ns]"),
876+
cp.array([True], dtype=np.bool_),
877+
cp.asarray(np.array([120], dtype="int64")).view("timedelta64[ns]"),
878+
cp.array([True], dtype=np.bool_),
879+
)
889880
assert int(out_v.get().view("int64")[0]) == 420
890881

891882

@@ -896,17 +887,24 @@ def test_masked_datetime_comparison(op):
896887
@cuda.jit(
897888
types.void(
898889
types.boolean[::1],
899-
_DT[::1],
890+
types.NPDatetime("ns")[::1],
900891
types.boolean[::1],
901-
_DT[::1],
892+
types.NPDatetime("ns")[::1],
902893
types.boolean[::1],
903894
)
904895
)
905896
def k(out, a, av, b, bv):
906897
out[0] = op(Masked(a[0], av[0]), Masked(b[0], bv[0])).value
907898

908899
out = cp.zeros(1, dtype=np.bool_)
909-
_launch(k, out, _dt_in([400]), _bool(True), _dt_in([1000]), _bool(True))
900+
_launch(
901+
k,
902+
out,
903+
cp.asarray(np.array([400], dtype="int64")).view("datetime64[ns]"),
904+
cp.array([True], dtype=np.bool_),
905+
cp.asarray(np.array([1000], dtype="int64")).view("datetime64[ns]"),
906+
cp.array([True], dtype=np.bool_),
907+
)
910908
assert bool(out.get()[0]) == op(400, 1000)
911909

912910

@@ -933,7 +931,14 @@ def k(out, a, av, b, bv):
933931
a = cp.asarray(np.array([1], dtype="int64")).view("datetime64[s]")
934932
b = cp.asarray(np.array([2], dtype="int64")).view("datetime64[ns]")
935933
out = cp.zeros(1, dtype=np.bool_)
936-
_launch(k, out, a, _bool(True), b, _bool(True))
934+
_launch(
935+
k,
936+
out,
937+
a,
938+
cp.array([True], dtype=np.bool_),
939+
b,
940+
cp.array([True], dtype=np.bool_),
941+
)
937942
# 1 second (1e9 ns) > 2 ns is True; a raw i64 compare would give 1 > 2 = False.
938943
assert bool(out.get()[0]) is True
939944

@@ -945,11 +950,11 @@ def test_masked_datetime_arith_validity_propagates():
945950

946951
@cuda.jit(
947952
types.void(
948-
_TD[::1],
953+
types.NPTimedelta("ns")[::1],
949954
types.boolean[::1],
950-
_DT[::1],
955+
types.NPDatetime("ns")[::1],
951956
types.boolean[::1],
952-
_DT[::1],
957+
types.NPDatetime("ns")[::1],
953958
types.boolean[::1],
954959
)
955960
)
@@ -958,15 +963,15 @@ def k(out_v, out_valid, a, av, b, bv):
958963
out_v[0] = m.value
959964
out_valid[0] = m.valid
960965

961-
out_v = _td_out()
966+
out_v = cp.zeros(1, dtype=np.int64).view("timedelta64[ns]")
962967
out_valid = cp.ones(1, dtype=np.bool_)
963968
_launch(
964969
k,
965970
out_v,
966971
out_valid,
967-
_dt_in([1000]),
968-
_bool(True),
969-
_dt_in([400]),
970-
_bool(False),
972+
cp.asarray(np.array([1000], dtype="int64")).view("datetime64[ns]"),
973+
cp.array([True], dtype=np.bool_),
974+
cp.asarray(np.array([400], dtype="int64")).view("datetime64[ns]"),
975+
cp.array([False], dtype=np.bool_),
971976
)
972977
assert bool(out_valid.get()[0]) is False

0 commit comments

Comments
 (0)