Clamp slice bounds in mixed fancy/slice indexing - #4397
Open
Adityaj0 wants to merge 1 commit into
Open
Conversation
mlx_gather_nd (used when a slice is combined with an array/int index, e.g. a[start:stop, idx_array]) only adjusted negative slice bounds by a single += axis_size and never clamped them into the valid [0, axis_size] range before passing them to arange(). This differs from the general slice() path in mlx/ops.cpp's normalize_slice(), which does clamp. As a result, out-of-range or heavily negative slice bounds (e.g. a[-100:4, idx] or a[0:200, idx] on a size-4 axis) produced arrays of the wrong shape filled with incorrect/garbage-repeated data instead of matching NumPy's clamping behavior, and extreme bounds (e.g. a[-10**9:4, idx]) could allocate huge bogus index arrays. Fix clamps start/end the same way normalize_slice does before building the arange, and adds a regression test.
Collaborator
|
Thanks for your contribution. Do you mind rewriting the description using provided template? |
nastya236
added
the
await response
Member
|
@Adityaj0 Please notice that not long ago we have added AI policy that now forbids automatic creating PRs from AI and requires disclosing usage of AI, with a simple pull request template that is required to be filled when AI is used: https://github.com/ml-explore/mlx/blob/main/.github/pull_request_template.md |
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.
Fixes #4399
When a slice is combined with an array/int index (e.g.
a[start:stop, idx_array]), the slice bounds were adjusted for negative indices but never clamped into the valid[0, axis_size]range before being passed toarange(). Out-of-range or heavily negative slice bounds produced arrays of the wrong shape containing bogus repeated/garbage data instead of matching NumPy's clamping behavior. Adds a regression test.