BUG: Fix buffered iterator stride after removing multi-index - #31985
Conversation
| if ((itflags & NPY_ITFLAG_HASMULTIINDEX) && | ||
| NAD_SHAPE(axisdata) == 1 && op_stride == 0) { | ||
| op_stride = NPY_MAX_INTP; | ||
| } |
There was a problem hiding this comment.
Just for a small contribution to the code review here -- I confirmed that both of the added regression tests fail locally on ARM Mac when I revert this source patch. The full testsuite is passing for me locally on this branch.
The patch does appear to be focused around a real world failure scenario, though it does seem a little obscure and hard to follow.
Probably a more active NumPy core dev should take a look--the patch is relatively small, but the team is getting pretty inundated at the moment.
seberg
left a comment
seberg
left a comment
There was a problem hiding this comment.
Thanks, looks right (wonder if we could merge it with the if/else above, but doesn't matter). Pretty bad embarrassing issue :(.
I'll approve, please go ahead to merge anyone (I'll very soon). I wish we could delay this part, but I don't think it is obvious.
|
I wish we could go back in time and remove or at least vastly simplify the python-level nditer. But we can't so in it goes. |
|
Thanks @snoopuppy582 |
5 participants
PR summary
This PR fixes a critical bug where incorrect coordinates are returned after the iterator buffer boundary for large
(N, 1)index arrays. As seen in real-world applications (e.g., MRI reconstruction inmckib2/pygrappa#107), this can silently produce incorrect results without raising any exceptions.Root Cause:
The issue occurs because the transfer loop is fixed to a zero stride for a size-one inner axis, failing to reflect the updated runtime stride after
remove_multi_indexis applied.Fix:
To resolve this, I modified
numpy/_core/src/multiarray/nditer_constr.cto exclude mutable strides from the fixed-stride optimization.Testing & Verification:
nditerregression tests innumpy/_core/tests/test_nditer.py.unravel_indexregression tests innumpy/lib/tests/test_index_tricks.py.Closes #31980
First time committer introduction
Hi, I am snoopuppy582, a developer using NumPy. I investigated this issue because of its silent but critical impact on real-world calculations, and I hope this patch helps improve the stability of the library.
AI Disclosure
I used AI tools (OpenAI Codex / ChatGPT) strictly for review and verification purposes to ensure the quality of the fix.
I manually analyzed the root cause, wrote the C modifications and regression tests, and executed all local validations. I have fully reviewed and understood the entire diff. Also, this PR description, the commit messages, and all future communications are written entirely by me manually.