Skip to content

Commit

Permalink
BUG: Fix that reducelikes honour out always (and live int he future)
Browse files Browse the repository at this point in the history
Reducelikes should have lived in the future where the `out` dtype
is correctly honoured always and used as one of the *inputs*.
However, when legacy fallback occurs, this leads to problems because
the legacy code path has 0-D fallbacks.

There are two probable solutions to this:
* Live with weird value-based stuff here even though it was never
  actually better especially for reducelikes.
  (enforce value-based promotion)
* Avoid value based promotion completely.

This does the second one, using a terrible hack by just mutating
the dimension of `out` to tell the resolvers that value-based logic
cannot be used.

Is that hack safe?  Yes, so long nobody has super-crazy custom type
resolvers (the only one I know is pyerfa and they are fine, PyGEOS
I think has no custom type resolver).
It also relies on the GIL of course, but...

The future? We need to ditch this value-based stuff, do annoying
acrobatics with dynamically created DType classes, or something similar
(so ditching seems best, it is topping my TODO list currently).

Testing this is tricky, running the test:
```
python runtests.py -t numpy/core/tests/test_ufunc.py::TestUfunc::test_reducelike_out_promotes
```
triggers it, but because reducelikes do not enforce value-based promotion
the failure can be "hidden" (which is why the test succeeds in a full test
run).

Closes gh-20739
  • Loading branch information
seberg authored and charris committed Jan 12, 2022
1 parent 4877d60 commit 75eb378
Showing 1 changed file with 18 additions and 0 deletions.
18 changes: 18 additions & 0 deletions numpy/core/src/umath/ufunc_object.c
Expand Up @@ -2718,6 +2718,21 @@ reducelike_promote_and_resolve(PyUFuncObject *ufunc,
* is NULL) so we pass `arr` instead in that case.
*/
PyArrayObject *ops[3] = {out ? out : arr, arr, out};

/*
* TODO: This is a dangerous hack, that works by relying on the GIL, it is
* terrible, terrifying, and trusts that nobody does crazy stuff
* in their type-resolvers.
* By mutating the `out` dimension, we ensure that reduce-likes
* live in a future without value-based promotion even when legacy
* promotion has to be used.
*/
npy_bool evil_ndim_mutating_hack = NPY_FALSE;
if (out != NULL && PyArray_NDIM(out) == 0 && PyArray_NDIM(arr) != 0) {
evil_ndim_mutating_hack = NPY_TRUE;
((PyArrayObject_fields *)out)->nd = 1;
}

/*
* TODO: If `out` is not provided, arguably `initial` could define
* the first DType (and maybe also the out one), that way
Expand All @@ -2738,6 +2753,9 @@ reducelike_promote_and_resolve(PyUFuncObject *ufunc,

PyArrayMethodObject *ufuncimpl = promote_and_get_ufuncimpl(ufunc,
ops, signature, operation_DTypes, NPY_FALSE, NPY_TRUE, NPY_TRUE);
if (evil_ndim_mutating_hack) {
((PyArrayObject_fields *)out)->nd = 0;
}
/* DTypes may currently get filled in fallbacks and XDECREF for error: */
Py_XDECREF(operation_DTypes[0]);
Py_XDECREF(operation_DTypes[1]);
Expand Down

0 comments on commit 75eb378

Please sign in to comment.