BUG: Fix that reduce-likes honor out always (and live in the future) #20805
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #20793.
Reducelikes should have lived in the future where the
outdtypeis 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:
actually better especially for reducelikes.
(enforce value-based promotion)
This does the second one, using a terrible hack by just mutating
the dimension of
outto tell the resolvers that value-based logiccannot 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:
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