Skip to content

Fall back to dense eigh in sparse_cholesky for rank-deficient PSD inputs#3308

Open
Transurgeon wants to merge 1 commit intomasterfrom
fix-rank-deficient-sparse-cholesky
Open

Fall back to dense eigh in sparse_cholesky for rank-deficient PSD inputs#3308
Transurgeon wants to merge 1 commit intomasterfrom
fix-rank-deficient-sparse-cholesky

Conversation

@Transurgeon
Copy link
Copy Markdown
Collaborator

Description

Please include a short summary of the change.
sparse_cholesky (added in #3070, partially patched in #3270) raises
ValueError: Cholesky factorization failed on simple rank-deficient PSD
inputs such as [[1, 1], [1, 1]]. The root cause is that QDLDL has no
pivoting and bails out as soon as a zero pivot appears mid-elimination —
which the existing rank-deficient handling in #3270 doesn't cover (it
only special-cases all-zero rows and zero pivots that land last in
QDLDL's elimination order).

This adds a dense numpy.linalg.eigh fallback inside
sparse_cholesky's existing except Exception branch. When QDLDL fails,
we eigendecompose A, classify it as PSD/NSD/indefinite using the same
tolerance logic as the QDLDL branch, and return an n × rank(A) sparse
factor built from the nonzero eigenpairs. The function's documented
contract (L[p,:] @ L[p,:].T == sign * A, k = rank(A)) now holds for
all PSD/NSD inputs.

Issue link (if applicable):

Type of change

  • New feature (backwards compatible)
  • New feature (breaking API changes)
  • Bug fix
  • Other (Documentation, CI, ...)

Contribution checklist

  • Add our license to new files.
  • Check that your code adheres to our coding style.
  • Write unittests.
  • Run the unittests and check that they’re passing.
  • Run the benchmarks to make sure your change doesn’t introduce a regression.

Comment thread cvxpy/utilities/linalg.py
# QDLDL has no pivoting and fails when a zero pivot appears
# mid-elimination on rank-deficient PSD/NSD inputs (e.g. [[1,1],[1,1]]).
# Fall back to a dense symmetric eigendecomposition.
w, V = np.linalg.eigh(A.toarray())
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The except Exception clause (unchanged from before) now triggers an O(n³) dense eigendecomposition + O(n²) memory allocation via A.toarray(), rather than just re-raising as a ValueError. If a large sparse matrix hits this path for an unexpected reason (not rank-deficiency), the dense fallback could OOM or hang.

Consider either:

  1. Narrowing the catch to the specific exception type QDLDL raises on zero-pivot (likely a C-level RuntimeError), or
  2. Adding a size guard before densifying, e.g.:
if n > 5000:
    raise ValueError(f"{SparseCholeskyMessages.FACTORIZATION_FAILED}: matrix too large for dense fallback")

This way truly unexpected failures on large matrices still surface immediately rather than silently attempting dense computation.

@odow
Copy link
Copy Markdown

odow commented Apr 29, 2026

This is the same saga as jump-dev/MathOptInterface.jl#2973 ultimately "fixed" jump-dev/MathOptInterface.jl#2967. Even then I don't like it. There's no magic fix. Every option has trade-offs.

We discussed doing this jump-dev/MathOptInterface.jl#2931 but decided not to merge because of the performance issues for larger models.

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