Fix vmap of partition/argpartition dropping the kth argument - #4116
Open
Adityaj0 wants to merge 1 commit into
Open
Fix vmap of partition/argpartition dropping the kth argument#4116Adityaj0 wants to merge 1 commit into
Adityaj0 wants to merge 1 commit into
Conversation
Partition::vmap and ArgPartition::vmap called the two-argument
partition(a, kth, stream) overload while passing the axis where kth
belongs. That overload partitions the *flattened* array, so under vmap
these ops silently returned a 1-D array partitioned around the wrong
element instead of a batched partition along the requested axis.
topk is implemented on top of partition, so it inherited the bug and
crashed under vmap ("[slice] Invalid number of indices or strides for
array with dimension 1") once the traced slice was applied to the
flattened result.
Pass kth_ explicitly so the (a, kth, axis, stream) overload is selected.
Adityaj0
force-pushed
the
fix-partition-vmap-kth
branch
from
August 10, 2026 02:49
2940021 to
9cd9263
Compare
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.
Fixes #4113.
Proposed changes
Partition::vmapandArgPartition::vmapcalled the two-argument overloadwhich is the one that partitions the flattened array, with
axis_ + axis_leftbound tokth.kth_was dropped entirely, so undervmapthese ops returned a 1-D array partitioned around the wrong element.Sort::vmapimmediately below is correct only becausesorthas nokthparameter.topkis built onpartition, so it inherited the bug and failed outright once the tracedslicewas applied to the flattened result.Passing
kth_explicitly selects the(a, kth, axis, stream)overload.Before, on
main:After:
Tests
Added
test_vmap_partitionandtest_vmap_topktopython/tests/test_vmap.py; there was previously novmapcoverage for these ops. They sweep everyin_axes, inneraxisand every validkth/k, comparing against the equivalent loop.Because
partitiononly pins thekthelement and leaves the two sides in an arbitrary order, the tests compare the sorted result against the sorted input and check thekthelement against the reference, rather than asserting element-wise equality that the API does not promise.argpartitionis checked by gathering with the returned indices.pre-commit run --all-filesto format my code and installed pre-commit prior to committing changesVerified with a CPU-only build (
-DMLX_BUILD_METAL=OFF);test_vmap,test_opsandtest_autogradpass.