Skip to content

Fix Metal sort for arrays with negative strides - #4226

Closed
reckylurker wants to merge 1 commit into
ml-explore:mainfrom
reckylurker:fix/metal-sort-negative-strides
Closed

Fix Metal sort for arrays with negative strides#4226
reckylurker wants to merge 1 commit into
ml-explore:mainfrom
reckylurker:fix/metal-sort-negative-strides

Conversation

@reckylurker

Copy link
Copy Markdown
Contributor

Fixes #4225.

Proposed changes

mx.sort, mx.topk, and mx.partition on Metal GPU returned zeros for non-first rows when called on arrays with negative strides (e.g. x[::-1, :]).

In single_block_sort and multi_block_sort, set_input_array binds Metal buffer 0 starting at a_buf + in.offset(). When non-sorted axes have negative strides, elem_to_loc in MSL computes negative relative pointer offsets for subsequent rows (tid.y = 1 -> -4 elements), attempting to read memory prior to Metal's bound buffer window. Metal's GPU memory protection traps these out-of-bounds reads and outputs 0.0f.

Matching NumPy's behavior when sorting negative-strided views, gpu_merge_sort now checks for negative strides and creates a temporary contiguous GPU copy via contiguous_copy_gpu(in, s). Zero-copy execution is fully preserved for standard contiguous arrays, transposed arrays (x.T), and positive strided views (x[::2, :]).

Added test_strided_negative_stride_sort in python/tests/test_ops.py.

Checklist

  • I have read the CONTRIBUTING document
  • I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the necessary documentation (if needed)

@reckylurker
reckylurker force-pushed the fix/metal-sort-negative-strides branch from ae419b3 to 4469d04 Compare August 13, 2026 06:32

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The bug is true but a correct fix should get the root cause fixed rather than working around the problem by doing a copy.

@zcbenz

zcbenz commented Aug 14, 2026

Copy link
Copy Markdown
Member

Closing in favor of #4252.

@zcbenz zcbenz closed this Aug 14, 2026
@reckylurker
reckylurker deleted the fix/metal-sort-negative-strides branch August 18, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] mx.sort / topk / partition on Metal GPU return zeros for rows with negative strides

2 participants