Skip to content

fix(cpu): name the right routine in LUF and SVD error messages - #4167

Merged
zcbenz merged 1 commit into
ml-explore:mainfrom
devteamaegis:fix/cpu-lapack-error-messages
Aug 11, 2026
Merged

fix(cpu): name the right routine in LUF and SVD error messages#4167
zcbenz merged 1 commit into
ml-explore:mainfrom
devteamaegis:fix/cpu-lapack-error-messages

Conversation

@devteamaegis

Copy link
Copy Markdown
Contributor

Proposed changes

What's broken. Two CPU linalg error messages name a LAPACK routine that did not run.

LUF::eval_cpu dispatches float32 and float64, but the failure message hardcodes the float32 routine, so a float64 failure reports sgetrf_ when dgetrf_ is what actually ran:

ss << "[LUF::eval_cpu] sgetrf_ failed with code " << info

SVD is wrong on three counts. It dispatches float32, float64 and complex64, but the message hardcodes the float32 prefix; it names gesvdx, which the file never calls (run() calls gesdd<T>); and it uses a svd_impl: prefix while every other message in the same file uses [SVD::eval_cpu], including one two lines above it:

ss << "svd_impl: sgesvdx_ failed with code " << info;

Observed on a CPU-only build, same wrong routine name for both dtypes:

$ python -c "import mlx.core as mx; mx.eval(mx.linalg.svd(mx.array([[float('nan'),1.0],[2.0,3.0]], dtype=mx.float32), stream=mx.cpu))"
... std::runtime_error: svd_impl: sgesvdx_ failed with code -4
$ python -c "import mlx.core as mx; mx.eval(mx.linalg.svd(mx.array([[float('nan'),1.0],[2.0,3.0]], dtype=mx.float64), stream=mx.cpu))"
... std::runtime_error: svd_impl: sgesvdx_ failed with code -4

For contrast, LAPACK itself reports the routine per dtype — mx.linalg.eig on the same input prints On entry to SHSEQR for float32 and DHSEQR for float64.

The fix. Describe the operation rather than naming a routine, which is what the other CPU linalg messages already do (Cholesky::eval_cpu "Cholesky decomposition failed", Eig/Eigh::eval_cpu "Eigenvalue decomposition failed", Inverse::eval_cpu "LU factorization failed"). That keeps the messages correct for every dtype without duplicating the dispatch. The svd_impl: prefix becomes [SVD::eval_cpu] to match its own file.

After:

... std::runtime_error: [SVD::eval_cpu] SVD failed with error code -4      # float32
... std::runtime_error: [SVD::eval_cpu] SVD failed with error code -4      # float64

On tests. These are message-only changes on error paths that cannot be reached from Python without terminating the process, so there is no test to add. The info < 0 branch in LUF means LAPACK got an illegal argument, which MLX never passes. The SVD branch is reachable (the repro above), but the throw happens inside encoder.dispatch, so it escapes the worker thread and calls std::terminate instead of surfacing as a Python exception — assertRaises cannot catch it. python/tests/test_linalg.py and test_ops.py are green (168 passed, 575 subtests).

That thread-propagation behaviour looks like a separate bug worth its own fix — mx.linalg.inv on a singular matrix aborts the interpreter the same way — but it needs a design decision about where to capture and rethrow, so I have left it out of this PR. Happy to file it as an issue if that is useful.

No benchmark: string literals only, no change to any code path.

Checklist

  • 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 — not applicable, see "On tests" above
  • I have updated the necessary documentation (if needed) — no API change

Both messages hardcoded the float32 routine name even though the ops
dispatch float32 and float64 (and complex64 for SVD), so a float64
failure reported a routine that never ran.

SVD was also naming the wrong routine entirely: it calls gesdd, not
gesvdx. Its two messages used a 'svd_impl:' prefix as well, while every
other message in the same file uses '[SVD::eval_cpu]'.

Describe the operation instead of naming the routine, which is what
Cholesky, Eig, Eigh and Inverse already do.
@zcbenz
zcbenz merged commit f66bcc9 into ml-explore:main Aug 11, 2026
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.

2 participants