Skip to content

Commit

Permalink
REV: Revert adding a default ufunc promoter
Browse files Browse the repository at this point in the history
Adding a default promoter should not be necessary with the
reduction-hack (use output DType to guess loop DType if incompatible
with the loop).

However, using it had two effects:
1. SciPy has a ufunc that has a loop it does not want to exist.
   This loop is homogeneous (unlike the correct one) and gives
   a deprecation warning.  The default promoter would assume the
   homogeneous loop is OK (maybe not ideal) if it exists.
   Since it is a "bad" loop that gives a deprecation warning, it
   is not really true though.
2. Datetime promotion is currently utterly buggy, leading to:
       timedelta.sum(dtype="f8")
   ignoring the `f8` requests.  But we actually end up relying on
   that behaviour in our `np.mean` implementation...
  • Loading branch information
seberg authored and charris committed Dec 9, 2021
1 parent cf2e656 commit 74f1d49
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 56 deletions.
68 changes: 13 additions & 55 deletions numpy/core/src/umath/ufunc_object.c
Expand Up @@ -2737,7 +2737,7 @@ reducelike_promote_and_resolve(PyUFuncObject *ufunc,
}

PyArrayMethodObject *ufuncimpl = promote_and_get_ufuncimpl(ufunc,
ops, signature, operation_DTypes, NPY_FALSE, NPY_FALSE, NPY_TRUE);
ops, signature, operation_DTypes, NPY_FALSE, NPY_TRUE, NPY_TRUE);
/* Output can currently get cleared, others XDECREF in case of error */
Py_XDECREF(operation_DTypes[1]);
if (out != NULL) {
Expand Down Expand Up @@ -5194,60 +5194,18 @@ PyUFunc_FromFuncAndDataAndSignatureAndIdentity(PyUFuncGenericFunction *func, voi
return NULL;
}
}

PyObject *promoter = NULL;
if (ufunc->ntypes == 1) {
npy_bool all_object = NPY_TRUE;
for (int i = 0; i < ufunc->nargs; i++) {
if (ufunc->types[i] != NPY_OBJECT) {
all_object = NPY_FALSE;
break;
}
}
if (all_object) {
promoter = PyCapsule_New(&object_only_ufunc_promoter,
"numpy._ufunc_promoter", NULL);
if (promoter == NULL) {
Py_DECREF(ufunc);
return NULL;
}
}
}
if (promoter == NULL && ufunc->nin > 1) {
promoter = PyCapsule_New(&default_ufunc_promoter,
"numpy._ufunc_promoter", NULL);
if (promoter == NULL) {
Py_DECREF(ufunc);
return NULL;
}
}
if (promoter != NULL) {
/* Always install default promoter using the common DType */
PyObject *dtype_tuple = PyTuple_New(ufunc->nargs);
if (dtype_tuple == NULL) {
Py_DECREF(promoter);
Py_DECREF(ufunc);
return NULL;
}
for (int i = 0; i < ufunc->nargs; i++) {
Py_INCREF(Py_None);
PyTuple_SET_ITEM(dtype_tuple, i, Py_None);
}
PyObject *info = PyTuple_Pack(2, dtype_tuple, promoter);
Py_DECREF(dtype_tuple);
Py_DECREF(promoter);
if (info == NULL) {
Py_DECREF(ufunc);
return NULL;
}

int res = PyUFunc_AddLoop((PyUFuncObject *)ufunc, info, 0);
Py_DECREF(info);
if (res < 0) {
Py_DECREF(ufunc);
return NULL;
}
}
/*
* TODO: I tried adding a default promoter here (either all object for
* some special cases, or all homogeneous). Those are reasonable
* defaults, but short-cut a deprecated SciPy loop, where the
* homogeneous loop `ddd->d` was deprecated, but an inhomogeneous
* one `dld->d` should be picked.
* The default promoter *is* a reasonable default, but switched that
* behaviour.
* Another problem appeared due to buggy type-resolution for
* datetimes, this meant that `timedelta.sum(dtype="f8")` returned
* datetimes (and not floats or error), arguably wrong, but...
*/
return (PyObject *)ufunc;
}

Expand Down
8 changes: 7 additions & 1 deletion numpy/core/tests/test_datetime.py
Expand Up @@ -2029,11 +2029,17 @@ def test_datetime_maximum_reduce(self):
assert_equal(np.maximum.reduce(a),
np.timedelta64(7, 's'))

def test_timedelta_correct_mean(self):
# test mainly because it worked only via a bug in that allowed:
# `timedelta.sum(dtype="f8")` to ignore the dtype request.
a = np.arange(1000, dtype="m8[s]")
assert_array_equal(a.mean(), a.sum() / len(a))

def test_datetime_no_subtract_reducelike(self):
# subtracting two datetime64 works, but we cannot reduce it, since
# the result of that subtraction will have a different dtype.
arr = np.array(["2021-12-02", "2019-05-12"], dtype="M8[ms]")
msg = r"ufunc 'subtract' did not contain a loop with signature "
msg = r"the resolved dtypes are not compatible"

with pytest.raises(TypeError, match=msg):
np.subtract.reduce(arr)
Expand Down

0 comments on commit 74f1d49

Please sign in to comment.