Bypass mx.vmap for natively batched MLX linalg ops - #2399
Open
guillaume-osmo wants to merge 2 commits into
Open
Conversation
) `mlx_funcify_SolveTriangular` read `op.lower` but never `op.unit_diagonal`, and `mx.linalg.solve_triangular` has no such argument, so the flag was dropped and the stored diagonal used instead of ones. Nothing raised. This also silently broke `lu_solve`, whose unit-triangular `L` is packed into the LU array with `U`'s diagonal in those slots, so any graph where `reuse_decomposition_multiple_solves` collapses two solves onto one factorization returned wrong numbers on MLX. Overwrite the diagonal with ones before the solve when the flag is set; this broadcasts over leading batch dims, so the batched case is covered too. `test_mlx_SolveTriangular` now parametrizes over `unit_diagonal`, and a new test pins the `lu_factor` + `lu_solve` path.
`Blockwise` is lowered with `mx.vmap`, which is not a safe wrapper for every MLX linalg primitive: * `mx.linalg.solve` and `mx.linalg.lu_factor` build an `LUF` primitive that has no batching rule, so every batched `solve`, `lu_factor` and `det` raised "[Primitive::vmap] Not implemented for LUF". * `mx.vmap`'s `solve_triangular` rule *drops the `upper` flag*. A batched triangular solve did not raise -- it silently solved the full system. The answer was only correct when the input happened to be exactly triangular, which is never the case for `cho_solve` or `lu_solve`, since `lu_factor` packs `L` and `U` into a single array. This was the more serious of the two: wrong numbers, no warning. MLX's own CPU-stream implementations already accept batched input, so add a `mlx_funcify_batched` registry that lets a core `Op` supply a natively batched implementation and skip `vmap` altogether. Registered for `Solve`, `SolveTriangular`, `CholeskySolve`, `LUFactor`, `Det` and `SLogDet`. Two details the registry has to handle: * `mx.linalg.solve` requires identical batch dims across inputs and, unlike `solve_triangular`, does not broadcast them, so inputs are aligned to a common batch shape first. * MLX only special-cases a 1-d `b`; with a batch it rejects `A (*batch, m, m)` against `b (*batch, m)`, so a vector right-hand side gets an explicit column axis. `_lu_det_parts` now reduces over the trailing axes only, so it accepts a stack of matrices. Verified against the reference for batch shapes `()`, `(4,)` and `(2, 3)` across solve (b_ndim 1 and 2), solve_triangular (both triangles, with and without `unit_diagonal`, on deliberately dense input), cho_solve, lu_solve, lu_factor, det, cholesky and inv, plus batch-dim broadcasting and gradients. Note `mx.vmap` handles `PivotToPermutations` correctly already, so it is deliberately left on the `vmap` path; a test pins that.
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 #2385.
Motivation
Blockwiseis lowered withmx.vmap, which is not a safe wrapper for every MLX linalg primitive. Two separate problems:1. Missing batching rule (the reported crash).
mx.linalg.solveandmx.linalg.lu_factorbuild anLUFprimitive that has novmaprule, so batchedsolve,lu_factoranddetall raise[Primitive::vmap] Not implemented for LUF. (detis not mentioned in the issue but goes throughlu_factor.)2.
mx.vmap'ssolve_triangularrule drops theupperflag. This one does not raise — it silently solves the full system:The vmapped answer is the full solve, exactly. It is only correct when the input happens to be exactly triangular — which
cho_solveandlu_solvenever are, sincelu_factorpacksLandUinto a single array. Wrong numbers, no warning; I consider this the more serious half.cholesky,invandPivotToPermutationsvmap correctly, so this is specific tosolve_triangular.Implementation
MLX's own CPU-stream implementations already accept batched input, so the
vmapwrapper is not needed for these in the first place. Add amlx_funcify_batchedsingledispatchregistry: a coreOpmay supply a natively batched implementation, andfuncify_Blockwisethen skipsvmapentirely.Registered for
Solve,SolveTriangular,CholeskySolve,LUFactor,DetandSLogDet._lu_det_partsnow reduces over the trailing axes only, so it accepts a stack of matrices.Two MLX details the registry has to absorb:
mx.linalg.solvedoes not broadcast batch dims (unlikesolve_triangular); it requires them to match. Inputs are aligned to a common batch shape first.b. With a batch it rejectsA (*batch, m, m)againstb (*batch, m), so a vector right-hand side gets an explicit trailing column axis, removed again on the way out.PivotToPermutationsis deliberately left on thevmappath, since its Python loop is bounded by the core length there and is already correct; a test pins that so this change cannot regress it.Tests
Verified against the reference for batch shapes
(),(4,)and(2, 3)acrosssolve(b_ndim1 and 2),solve_triangular(both triangles, with and withoutunit_diagonal, on deliberately dense input),cho_solve,lu_solve,lu_factor,det,choleskyandinv, plus batch-dim broadcasting and gradients.New tests, all of which fail without this change:
test_mlx_solve_batched,test_mlx_solve_batch_broadcasttest_mlx_SolveTriangular_batched_ignores_other_triangle— pins the silent corruption abovetest_mlx_lu_solve_batched,test_mlx_lu_factor_batched,test_mlx_det_batchedtest_mlx_pivot_to_permutations_batched— passes before and after, pinning thevmappath that is intentionally keptFull
tests/link/mlx/suite: no regressions.Note
Every
mx.linalg.*call in this backend runs onstream=mx.cpu, so none of this is GPU work — relevant to #2383.