Fix dpnp.insert ignoring out-of-bounds negative indices in multi-element obj - #3041
Merged
Conversation
Contributor
|
Array API standard conformance tests for dpnp=0.21.0dev7=py314ha0e2e8e_16 ran successfully. |
Collaborator
|
No base build to compare — fix-insert-oob-negative-indices into master |
Contributor
|
View rendered docs @ https://intelpython.github.io/dpnp/index.html |
antonwolfy
marked this pull request as ready for review
August 26, 2026 11:16
antonwolfy
requested review from
ndgrigorian and
vlad-perevezentsev
as code owners
August 26, 2026 11:16
…ent obj Backport of numpy#31782. The multi-element index path in _insert_array_indices normalized negative indices without any bounds check, so an out-of-bounds negative index mixed with in-bounds ones silently produced wrong results instead of raising. Add the same bounds validation numpy uses so any out-of-bounds index raises IndexError, and extend the test to cover the mixed case.
Parametrize the out-of-bounds test over both numpy and dpnp and add multi-element cases that exercise the newly added bounds check in _insert_array_indices, including the positive (max > n) branch, plus an ND case validating axis/size reporting for a non-zero axis.
antonwolfy
force-pushed
the
fix-insert-oob-negative-indices
branch
from
August 26, 2026 17:57
68c84e6 to
97b4495
Compare
Move the out-of-bounds validation out of _insert_array_indices into a shared _check_index_bounds helper called from insert(). When obj lives on the host (Python sequence/scalar or NumPy array) the bounds are checked with NumPy, avoiding any device sync; a slice is skipped since it is always in bounds; only a device-array obj needs a single host transfer, reading both extremes at once instead of two.
A boolean mask obj passed as a host object (Python sequence or NumPy array) was validated on its raw boolean values instead of the selected positions, so an oversized mask could pick indices beyond the axis and silently produce a wrong result. Flatnonzero the host mask before the bounds check, matching NumPy and the device path. Add regression tests covering host list, host NumPy, and device dpnp masks.
vlad-perevezentsev
self-requested a review
August 31, 2026 13:25
vlad-perevezentsev
approved these changes
Aug 31, 2026
vlad-perevezentsev
left a comment
vlad-perevezentsev
left a comment
Contributor
There was a problem hiding this comment.
LGTM
Thank you @antonwolfy
github-actions Bot
added a commit
that referenced
this pull request
Aug 31, 2026
…ent obj (#3041) `dpnp.insert` has two index paths: the singleton path (`_insert_singleton_index`) already validated bounds, but the multi-element array path (`_insert_array_indices`) normalized negative indices with `indices[indices < 0] += n` without any bounds check. As a result, an out-of-bounds negative index mixed with in-bounds ones — e.g. `dpnp.insert([0, 1, 2], [-6, 0], [9, 8])` — silently produced a wrong result instead of raising. This PR adds the same bounds validation NumPy introduced: if any index is `< -n` or `> n`, `IndexError` is raised with the standard `index {i} is out of bounds for axis {axis} with size {n}` message. In-bounds and out-of-bounds indices mixed in a single call now consistently raise. 1d7c6fa
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
dpnp.inserthas two index paths: the singleton path (_insert_singleton_index) already validated bounds, but the multi-element array path (_insert_array_indices) normalized negative indices withindices[indices < 0] += nwithout any bounds check. As a result, an out-of-bounds negative index mixed with in-bounds ones — e.g.dpnp.insert([0, 1, 2], [-6, 0], [9, 8])— silently produced a wrong result instead of raising.This PR adds the same bounds validation NumPy introduced: if any index is
< -nor> n,IndexErroris raised with the standardindex {i} is out of bounds for axis {axis} with size {n}message. In-bounds and out-of-bounds indices mixed in a single call now consistently raise.