BUG: incorrect temp elision for new-style (NEP 43) user-defined dtypes by MaartenBaert · Pull Request #31193 · numpy/numpy

MaartenBaert · 2026-04-10T12:03:39Z

PR summary

can_elide_temp() in temp_elide.c incorrectly identifies new-style user-defined dtypes as numeric types eligible for in-place buffer reuse. For large arrays this silently rewrites a*a + b*b into (a*a) += (b*b), which raises a TypeError when the result dtype of the in-place add does not match the pre-allocated buffer.

I encountered this bug while implementing a custom fixed-point dtype, which automatically increases its bit width on every operation as required to avoid overflow. Since addition increases the bit width by one, attempting to reuse the temporary buffer results in an incompatible destination dtype being provided to the add ufunc and raises a TypeError.

The root cause is that PyArray_ISNUMBER evaluates to true for new-style user dtypes since it checks (type) <= NPY_CLONGDOUBLE and type_num = -1 for these types.

This could be fixed either in PyArray_ISNUMBER (probably more correct, but may have unexpected side effects) or with a minimal extra check in can_elide_temp (the approach used by the PR). Let me know if you'd like me to fix PyArray_ISNUMBER instead.

AI Disclosure

This bug was identified and fixed by Claude Sonnet 4.6, and then reviewed by me.
Full AI-generated explanation: https://gist.github.com/MaartenBaert/7540176c4005d26a0b292baefbec8519

MaanasArora · 2026-04-10T12:53:33Z

Thanks, seems right! FWIW I think it is OK to change PyArray_ISNUMBER at the level of PyTypeNum_ISNUMBER (doing it locally for me didn't break NumPy tests at least, and type number checks can often be legacy). If it breaks any code it is probably unearthing bugs. That said not entirely familiar with these macros so maybe wait for someone else.

seberg · 2026-04-10T16:00:14Z

Nice, thanks for looking into this! I think it is probably good enough to reject only parametric dtypes here? I.e. with other dtypes we assume that things are fine.

MaartenBaert · 2026-04-11T09:09:01Z

Just pushed a new fix which doesn't check type_num but instead checks for presence of the NPY_DT_PARAMETRIC flag. I'm checking the flag manually rather than through NPY_DT_is_parametric to avoid pulling in dtypemeta.h.

seberg · 2026-04-11T09:19:39Z

I am sorry, I remembered last night late and re-remembering only right now... But what slipped my mind completely is that the Py_NUMBER macro was just wrong, it should include the > 0 check that you correctly added.
I.e. I think your solution was already better than what I said, just not quite the best spot :(.

(I would be happy to broadening it up to the newer NPY_DT_is_numeric() && !parametric, but I am not fully confident that it is guaranteed correct always, it might need a flag to say "go ahead an use in-place for homogeneous operations".)

EDIT: If it's annoying, I may just push it later.

MaartenBaert · 2026-04-11T10:30:16Z

No problem, I will try changing PyTypeNum_ISNUMBER and let you know what happens.
Regarding the parametric check, I'm actually not sure whether it is appropriate to assume that all non-parametric types can use the temp elision optimization, since you can't really predict what kind of conventions custom dtypes may choose to use. E.g. consider a datetime-like dtype where the minus operator produces a time delta type instead.

MaartenBaert · 2026-04-11T10:44:36Z

The macro fix seems to work equally well, both for my custom dtype test suite and the numpy test suite.

seberg · 2026-04-11T12:24:56Z

Thanks. Yeah, it seems like anything would be guess-work, while usually true, weird promotion could always happen.

@SwayamInSync maybe you have time for a quick look. I pushed one more hot-fix for the single case that looked like it might cause regressions here. I.e. that quaddtype_arr.conjugate() works via the ufunc.

Now, it seems for NumPy conjugate() actually just returns self (although I think a view should be OK). But, I think that fix isn't back-portable.
I have tested it locally, we could add a test, but it wouldn't run anyway for the time being.

For a better fix, I think we could:

Move the numeric check first, to error out better (but broaden it to actually use the new definition that user-dtypes can match). -- maybe not strictly necessary.
Check if .imag has a definition. If it does use the ufunc path.

The above would mean that quaddtype_arr.conjugate() would return self. That is a theoretical regression for user defined new-style complex dtypes.
Since I think we can be very confident those don't actually exist right now, I think that is fine.

MaartenBaert · 2026-04-11T12:29:44Z

Now, it seems for NumPy conjugate() actually just returns self (although I think a view should be OK). But, I think that fix isn't back-portable.

That's related to a different issue which also affects my complex fixed-point dtype: there is no way to inform numpy that my type is complex, so it assumes it is real and uses the trivial implementation of .real (return self), .imag (return zero) and .conj() (return self). I wouldn't mind making a fix for that too, but would need some guidance on how to proceed. This might require another dtype flag?

MaanasArora · 2026-04-11T12:37:14Z

That's related to a different issue which also affects my complex fixed-point dtype: there is no way to inform numpy that my type is complex

#30984 included ufuncs for real and imag, so for now you can register them via PyUFunc_AddLoopsFromSpecs as in the docs. These would be then used for your dtype. A flag to use defaults would still be nice though.

MaartenBaert · 2026-04-11T12:44:10Z

Good to know! Is there a similar ufunc for conj?

MaanasArora · 2026-04-11T12:51:19Z

Yes pretty sure "conjugate" is the one! That is a very "real" ufunc so you should be able to register it directly even IIRC.

SwayamInSync · 2026-04-13T10:54:30Z

 {
    if (PyArray_ISCOMPLEX(self) || PyArray_ISOBJECT(self) ||
-            PyArray_ISUSERDEF(self)) {
+            PyArray_ISUSERDEF(self) || !NPY_DT_is_legacy(PyArray_DESCR(self))) {


Thanks @seberg this works well. LGTM
Also noticed in quaddtype's we don't have test for quad_arr.conj() (we use np.conj which directly through the ufunc dispatch machinery and works)

seberg · 2026-04-13T11:23:23Z

Thanks for having a look @SwayamInSync, I'll put this in then.

I'll try to follow up with something that makes this work better for quaddtype in the future (e.g. also returning a view).

I think we can backport this @charris, but if it doesn't matter to @MaartenBaert then I don't mind either way.
(There is a small risk I missed another case like the .conj() one, but I don't think so and it would be even more niche probably.)

#31193) Co-authored-by: Maarten Baert <maarten.baert@keysight.com> Co-authored-by: Sebastian Berg <sebastianb@nvidia.com>

BUG: incorrect temp elision for new-style (NEP 43) user-defined dtypes (#31193)

jorenham added the 00 - Bug label Apr 10, 2026

jorenham changed the title ~~Fix incorrect temp elision for new-style (NEP 43) user-defined dtypes~~ BUG: incorrect temp elision for new-style (NEP 43) user-defined dtypes Apr 10, 2026

MaartenBaert force-pushed the main branch from 59214f0 to 1e1b490 Compare April 11, 2026 09:04

Fix incorrect temp elision for new-style (NEP 43) user-defined dtypes

7b1ef89

MaartenBaert force-pushed the main branch from 1e1b490 to 7b1ef89 Compare April 11, 2026 10:43

Prevent regression for conjugate one quaddtype (mostly/only)

58192e8

SwayamInSync approved these changes Apr 13, 2026

View reviewed changes

SwayamInSync mentioned this pull request Apr 13, 2026

[TEST] Adding test for quad_arr.conj() numpy/numpy-quaddtype#81

Merged

seberg merged commit 7a0dfad into numpy:main Apr 13, 2026
86 checks passed

seberg added the 09 - Backport-Candidate PRs tagged should be backported label Apr 13, 2026

charris mentioned this pull request Apr 25, 2026

BUG: incorrect temp elision for new-style (NEP 43) user-defined dtypes (#31193) #31329

Merged

charris removed the 09 - Backport-Candidate PRs tagged should be backported label Apr 25, 2026

charris pushed a commit that referenced this pull request Apr 25, 2026

BUG: incorrect temp elision for new-style (NEP 43) user-defined dtypes (

69022a9

#31193) Co-authored-by: Maarten Baert <maarten.baert@keysight.com> Co-authored-by: Sebastian Berg <sebastianb@nvidia.com>

charris added a commit that referenced this pull request Apr 25, 2026

Merge pull request #31329 from charris/backport-31193

b3ecd15

BUG: incorrect temp elision for new-style (NEP 43) user-defined dtypes (#31193)

Uh oh!

Uh oh!

Conversation

MaartenBaert commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR summary

AI Disclosure

Uh oh!

MaanasArora commented Apr 10, 2026

Uh oh!

seberg commented Apr 10, 2026

Uh oh!

MaartenBaert commented Apr 11, 2026

Uh oh!

seberg commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaartenBaert commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaartenBaert commented Apr 11, 2026

Uh oh!

seberg commented Apr 11, 2026

Uh oh!

MaartenBaert commented Apr 11, 2026

Uh oh!

MaanasArora commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaartenBaert commented Apr 11, 2026

Uh oh!

MaanasArora commented Apr 11, 2026

Uh oh!

SwayamInSync Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

seberg commented Apr 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

MaartenBaert commented Apr 10, 2026 •

edited

Loading

seberg commented Apr 11, 2026 •

edited

Loading

MaartenBaert commented Apr 11, 2026 •

edited

Loading

MaanasArora commented Apr 11, 2026 •

edited

Loading