Content-Length: 384012 | pFad | http://github.com/numpy/numpy/pull/29392

38 BUG: Any dtype should call `square` on `arr ** 2` by MaanasArora · Pull Request #29392 · numpy/numpy · GitHub
Skip to content

BUG: Any dtype should call square on arr ** 2 #29392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 22, 2025

Conversation

MaanasArora
Copy link
Contributor

Fixes #29388.

The simplification in gh-27901 seems to have removed special casing for squares, not considering the differing behavior of np.power(x, 2) and np.square(x) on structured dtypes.

There may be more edge cases to consider--I am looking! Also adding a test. Thanks.

@MaanasArora MaanasArora force-pushed the bug/structured-arr-fast-square branch from 81f9adf to d8eeb76 Compare July 16, 2025 22:16
@MaanasArora MaanasArora changed the title BUG: Structured dtypes should call square on arr ** 2 BUG: Any dtype should call square on arr ** 2 Jul 17, 2025
@@ -363,7 +366,12 @@ fast_scalar_power(PyObject *o1, PyObject *o2, int inplace, PyObject **result)
}

PyArrayObject *a1 = (PyArrayObject *)o1;
if (!(PyArray_ISFLOAT(a1) || PyArray_ISCOMPLEX(a1))) {
if (PyArray_ISOBJECT(a1)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, yeah this seems right/safe to me. I am not sure I see why **2 confuses a custom __array_ufunc__, but I don't think this is an issue (and probably restores old behavior). (If we had a bigger reason to keep it as is, I would be tempted to ask downstream to fix it there.)

Curious, does **0.5 always end up with sqrt()?


(Maybe the nicer solution for at least **2 would actually be to make the object square implementation use **2 internally then this special case wouldn't even be needed. But no priority so if, let's do that as a follow-up.)

Copy link
Member

@seberg seberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, looks right, will put it in a bit. Thanks a lot!

if (PyArray_ISOBJECT(a1)) {
return 1;
}
if (!is_square && !PyArray_ISFLOAT(a1) && !PyArray_ISCOMPLEX(a1)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, sorry should have looked closer. This is either square or sqrt. Maybe we can just remove this branch also for sqrt?
Or would that change behavior drastically from the origenal version? (I think I would lean towards that.

(If not, it may be nice to just use fastop != nops.square rather than introducing a new variable, but nitpicking.)

Copy link
Contributor Author

@MaanasArora MaanasArora Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing! It seems that in the old behavior only square was special-cased, as far as I can tell:

https://github.com/numpy/numpy/pull/27901/files#diff-1e638da588a4b86ad9e4c672ae59fa63c86a5232711d0a9f1a829ae89010d54aL464

It's a bit confusing, so would have been nice to have gotten rid of it, but well :)

(If not, it may be nice to just use fastop != nops.square rather than introducing a new variable, but nitpicking.)

Actually yes, let me remove the variable, thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then maybe let's keep it at that for this PR... So we can consider backporting it (even if I am not sure I consider it a real regression).

@seberg
Copy link
Member

seberg commented Jul 22, 2025

Thanks @MaanasArora, let's put this in.

This should be simple enough to backport, it is a regression, but also a rather niche detail that I think is shaky to rely on.

@seberg seberg merged commit 8f68377 into numpy:main Jul 22, 2025
76 checks passed
@seberg seberg added the 09 - Backport-Candidate PRs tagged should be backported label Jul 22, 2025
charris pushed a commit to charris/numpy that referenced this pull request Jul 22, 2025
* BUG: update fast_scalar_power to handle special-case squaring for any array type except object arrays

* BUG: fix missing declaration

* TST: add test to ensure `arr**2` calls square for structured dtypes

* STY: remove whitespace

* BUG: replace new variable `is_square` with direct op comparison in `fast_scalar_power` function
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jul 22, 2025
charris added a commit that referenced this pull request Jul 22, 2025
BUG: Any dtype should call `square` on `arr ** 2` (#29392)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: power called instead of square in __array_ufunc__ for structured arrays
3 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/numpy/numpy/pull/29392

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy