Skip to content

Add bool to Metal sort kernel instantiations - #4861

Open
utsav1033 wants to merge 1 commit into
ml-explore:mainfrom
utsav1033:fix-gpu-bool-sort
Open

utsav1033 wants to merge 1 commit into
ml-explore:mainfrom
utsav1033:fix-gpu-bool-sort

Conversation

@utsav1033

Copy link
Copy Markdown
  • ☑️ I understand it is strictly prohibited to use AI to write PR description
  • AI usage disclosure: I used AI to find the bug with a CPU/GPU comparison sweep, it built MLX and ran tests and gave me the test code.

Proposed changes

Sorting a bool array on the GPU fails, while the same call works on the CPU:

import mlx.core as mx

a = mx.array([True, False, True, False])
mx.sort(a, stream=mx.cpu)  # works
mx.sort(a, stream=mx.gpu)  # fails: Unable to load kernel c_block_sort_bool__bool__bn32_tn4

The cause is that sort.metal instantiates the sort kernels for every dtype except bool, so no bool kernel is ever built. This PR adds bool_ to both instantiation lists:

  • the single-block sort, used for small rows
  • the multi-block sort, used for large rows (sorted in chunks, then merged)

i didnt change anything else as sorting relies on comparison and its worknig for bool(false<true), and on a padding value, which in turn exists as Limits::max = true.

argsort, topk and partition use the same kernels, so this fixes them for bool too.

Testing

  • Added test_sort_bool to test_ops.py. It fails before this change and passes after.
  • Checked against the CPU and numpy for sizes from 3 to 1M, which covers both the single-block and multi-block paths. The checks covered the argsort indices, topk, partition, 2D and sliced inputs, and all-True, all-False and empty arrays.
  • All 177 tests in test_ops.py pass, and the formatting checks pass.

Tested on my Apple M4, on current main

This branch has not been deployed

No deployments
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.

1 participant