Skip to content

BUG: Fix that reduce-likes honor out always (and live in the future)#20805

Merged
charris merged 1 commit intonumpy:maintenance/1.22.xfrom
charris:backport-20793
Jan 13, 2022
Merged

BUG: Fix that reduce-likes honor out always (and live in the future)#20805
charris merged 1 commit intonumpy:maintenance/1.22.xfrom
charris:backport-20793

Conversation

@charris
Copy link
Copy Markdown
Member

@charris charris commented Jan 12, 2022

Backport of #20793.

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

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 numpygh-20739
@charris charris added 00 - Bug 08 - Backport Used to tag backport PRs labels Jan 12, 2022
@charris charris added this to the 1.22.1 release milestone Jan 12, 2022
@charris charris merged commit bd8ade5 into numpy:maintenance/1.22.x Jan 13, 2022
@charris charris deleted the backport-20793 branch January 13, 2022 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

00 - Bug 08 - Backport Used to tag backport PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants