[BUG] LAPACK `info` is not negative

[NAThompson]

Describe the bug

When a non-positive semidefinite matrix is passed into the Cholesky decomposition, the info is 2, not a negative number.

To Reproduce

Apply the following patchfile:

diff --git a/mlx/backend/common/cholesky.cpp b/mlx/backend/common/cholesky.cpp
index 62807e6..3f4beeb 100644
--- a/mlx/backend/common/cholesky.cpp
+++ b/mlx/backend/common/cholesky.cpp
@@ -5,6 +5,7 @@
 #include "mlx/backend/common/lapack.h"
 #include "mlx/linalg.h"
 #include "mlx/primitives.h"
+#include <iostream>

 namespace mlx::core {

@@ -51,6 +52,8 @@ void cholesky_impl(const array& a, array& factor, bool upper) {
       throw std::runtime_error(msg.str());
     }

+    std::cout << "Cholesky info = " << info << std::endl;
+
     // Zero out the upper/lower triangle while advancing the pointer to the
     // next matrix at the same time.
     for (int row = 0; row < N; row++) {

And run this code:

#!/usr/bin/env python3

import mlx
import mlx.core

A = mlx.core.array([[1.0, 2.0], [2.0, 1.0]])
print(f"Input A = {A}")
L = mlx.core.linalg.cholesky(A, stream=mlx.core.cpu)
print(f"L={L}")

A_recovered = [email protected]
print(f"L@L^T = {A_recovered}")

Info = 2 will be printed.

Expected behavior

The comment mlx/backend/common/cholesky.cpp in cholesky_impl (confusingly) claim it does not and should not throw, but then it explicitly throws on info < 0:

    // TODO: We do nothing when the matrix is not positive semi-definite
    // because throwing an error would result in a crash. If we figure out how
    // to catch errors from the implementation we should throw.
    if (info < 0) {
      std::stringstream msg;
      msg << "[cholesky] Cholesky decomposition failed with error code "
          << info;
      throw std::runtime_error(msg.str());
    }

However, this condition is never satisfied, as error states are communicated with positive integers. Hence the desired fix is not quite obvious-should the error check condition be fixed and the throw be maintained?

[angeloskath]

As the comment above if (info < 0) there was a decision made to ignore the case where the matrix is not positive semi-definite because the only other option is to crash the program unfortunately.

In the case of info < 0 then it was an actual runtime error so it makes a bit of sense to crash.

You can also see the discussion at #1743 for more details.