Add mx.searchsorted - #4014
Closed
erwinzhang7 wants to merge 1 commit into
Closed
Conversation
Implements the binary search requested in ml-explore#1255, composed from existing primitives rather than as a new primitive, so it works on CPU, Metal and CUDA without any new kernels. Two forms are used. Counting matches with a broadcast comparison is cheapest while the n by values.size mask stays small, and a branchless binary search is used past that, where the mask becomes both slow and a memory problem. The threshold is 4M elements, chosen from the benchmark in the pull request description. The binary search accumulates into a single running result rather than tracking a lower and upper bound, which avoids the two converging and stepping past each other and keeps every gather in range.
zcbenz
reviewed
Aug 6, 2026
zcbenz
left a comment
Member
There was a problem hiding this comment.
Thanks for the PR but this is something that we want to have a kernel rather than a fallback. Sometimes we do add fallback implementations when it is absolutely necessary, but I don't think this is the case.
Author
|
Noted, thanks for taking a look. Working on a real kernel version instead, with CPU, Metal and CUDA implementations. Will open a fresh PR once it is ready rather than reviving this one. |
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
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.
Closes #1255.
Adds
mx.searchsorted(sorted_sequence, values, side="left"), matchingnp.searchsortedsemantics.Approach
Composed from existing primitives in
ops.cpprather than added as a new primitive. That means no new Metal or CUDA kernels, and it works on every backend immediately. It is also the approach originally suggested in the issue.This is deliberately different from the earlier attempt in #2817, which added a primitive with a CPU implementation, left GPU evaluation as a TODO, and came to 901 lines across 13 files before being closed. The whole change here is 170 lines including tests and docs.
Which algorithm, and the dispatch
The issue left an open question: whether to use the linear form, the binary search, or dispatch between them. The suggestion at the time was to measure and pick. Here are the numbers on an M5 Max, current main, bf16 aside and all float32:
Milliseconds. The binary search is close to flat across a 2000x range in
n, while the linear form is cheaper at small sizes and then degrades badly and eventually cannot allocate. The crossover sits between 4M and 33M for then * mproduct, so the implementation dispatches at 4M.That gives the better of the two everywhere. Against the workaround people are currently copying out of the issue thread:
mx.searchsortedBelow the threshold it is the same computation, so parity is expected. Note the last row allocates a 34 billion element mask for the workaround, which only completes at all because this machine has 128 GB.
All timings warm up and measure by wall clock rather than by iteration count. With a fixed iteration count these kernels are short enough that the measurement tracks the GPU clock ramp instead of the kernel, and the same config can drift by 3x between processes.
Implementation note
The binary search accumulates into a single running result:
rather than tracking a lower and upper bound. The pair form needs an extra guard, because once lower and upper converge the midpoint update can push lower past upper, and the gather can index
n. Accumulating avoids both without the extra mask.Semantics
Validated against
np.searchsortedon 178 cases before writing any C++, then again through the built op on 44 more, forcing both dispatch branches and running on both the GPU and CPU streams. Covers both sides, duplicates, exact matches, values below and above the range,n = 1, integer and mixed dtypes, and multi dimensionalvalues, plus an exhaustive check for everynfrom 1 to 32 against every gap and every exact value.Returns
uint32to matchargsort,argmaxandargpartition.Scope
sorted_sequencemust be 1-D, which matchesnp.searchsorted. Batched sorted sequences, astorch.searchsortedsupports, are not included here. That would slot in as atake_along_axisvariant if wanted, but I kept this to the shape the issue asked for rather than widening it.