Skip to content

Fix mx.sort vjp to transpose the permutation - #3700

Merged
angeloskath merged 1 commit into
ml-explore:mainfrom
obchain:fix/sort-vjp-permutation-transpose
Jun 17, 2026
Merged

angeloskath merged 1 commit into
ml-explore:mainfrom
obchain:fix/sort-vjp-permutation-transpose

Conversation

@obchain

@obchain obchain commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

Closes #3699.

Sort::vjp delegated to Sort::jvp, but sorting applies a permutation P to the input and a permutation matrix is not symmetric. The jvp gathers the tangents along the argsort indices (P v); the vjp must scatter the cotangents back to the original positions (P^T w). Delegating to the jvp routed the cotangents the wrong way, giving incorrect gradients for any input whose sort permutation is not an involution. This affects CPU and GPU (the rule is in primitives.cpp), and there were no autograd tests for sort, so it went unnoticed.

The fix computes the vjp with put_along_axis on the argsort indices, mirroring the existing, correct Partition::vjp.

import mlx.core as mx
x = mx.array([3.0, 1.0, 2.0, 5.0, 4.0])
cotan = mx.array([10.0, 20.0, 30.0, 40.0, 50.0])
print(mx.vjp(lambda a: mx.sort(a), (x,), (cotan,))[1][0])
# before: [20, 30, 10, 50, 40]   after: [30, 10, 20, 50, 40] (matches finite differences)

Adds test_sort_grad covering the exact gradient and the jvp/vjp adjoint identity across axes.

Checklist

Put an x in the boxes that apply.

  • 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)

Sort::vjp delegated to Sort::jvp, but sorting applies a permutation to
the input and a permutation matrix is not symmetric. The jvp gathers the
tangents along argsort (P v); the vjp must scatter the cotangents back to
the original positions (P^T w) instead. Delegating to the jvp routed the
cotangents the wrong way, producing incorrect gradients for any input
whose sort permutation is not an involution.

Compute the vjp with put_along_axis on the argsort indices, mirroring the
existing Partition::vjp. Add an autograd test covering the value and the
jvp/vjp adjoint identity across axes.

@angeloskath angeloskath 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.

Thanks for catching this!

@angeloskath
angeloskath merged commit d96b202 into ml-explore:main Jun 17, 2026
16 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Jul 7, 2026
1 task done
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 vjp returns wrong gradients (delegates to jvp without transposing the permutation)

2 participants