Expose CHOLMOD's reciprocal condition number estimate as CHOLMOD.rcond - #776
Open
ViralBShah wants to merge 1 commit into
Open
Expose CHOLMOD's reciprocal condition number estimate as CHOLMOD.rcond#776ViralBShah wants to merge 1 commit into
CHOLMOD.rcond#776ViralBShah wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #776 +/- ##
=======================================
Coverage 92.06% 92.06%
=======================================
Files 12 12
Lines 8377 8379 +2
=======================================
+ Hits 7712 7714 +2
Misses 665 665 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`cholmod_rcond` was already wrapped in `wrappers.jl` but unreachable from Julia. Add a thin `rcond(F::Factor)` in the index-type loop and document what the estimate actually is: min/max of the factor diagonal, squared for `LL'`, so that `cholesky` and `ldlt` of the same matrix agree. It is exact for diagonal matrices and an upper bound on `1 / cond(A, 2)` otherwise -- verified over 300 random SPD matrices, where it never dipped below the true value and was up to 91x optimistic. Documented as a way to detect a badly conditioned or singular factorization rather than to measure conditioning. Fixes #118. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L8HenXTVrASo1sBZVGhpDQ
ViralBShah
force-pushed
the
cholmod-rcond
branch
from
September 8, 2026 14:06
021f76b to
09be1ea
Compare
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.
Fixes #118.
cholmod_rcondwas already wrapped insrc/solvers/wrappers.jl(both the_l_and 32-bit variants), it just had no Julia-level caller, so thefunctionality was unreachable. This adds a thin
rcond(F::CHOLMOD.Factor)inthe index-type loop next to
check_factor, plus a docstring, apublicdeclaration, an entry in the solvers API page, and tests.
What the estimate is
CHOLMOD computes it from the factor diagonal alone — smallest
absentry overlargest — squared for an
LL'factorization, so thatcholeskyandldltofthe same matrix agree:
(
diagof the Cholesky factor is[1, √2, 2], so(1/2)^2;diagof theLDL'factor is[1, 2, 4], so1/4unsquared.)How good it is
Worth being explicit about in the docstring, since "condition number" invites
the assumption of a norm-based estimate. Over 300 random sparse SPD matrices I
compared it against the true
1 / cond(Matrix(A), 2):interlacing of Cholesky pivots between
λminandλmax;So it is exact for diagonal matrices and an upper bound in general — safe for
detecting a badly conditioned or singular factorization, not for measuring
conditioning. The docstring says so.
API choice
I did not overload
LinearAlgebra.condforCHOLMOD.Factor.cond(A, 1)and
cond(A, Inf)already exist forAbstractSparseMatrixCSCviaopnormestinv, and they are norm-based estimates of a different quality; havingcond(F)silently return1/(a diagonal-ratio estimate) seemed likely tomislead. Exposing it under its own name matches what the issue asked for
(access to
cholmod_rcond) and leavescondfree if a real norm-based estimatefor factorizations is wanted later. Happy to change if you would rather have
cond(F).Testing
New testset runs for every
Tv×Ticombination the CHOLMOD tests alreadycover (
Float32/Float64×Int32/Int64), 5 tests each: the exact diagonalcase for both
choleskyandldlt, the 1-by-1 case (1.0), the singular case(
0.0, viacheck=false), and the bound against1 / cond(A, 2). The fulltest/cholmod.jlpasses with no failures, and the new jldoctest passes underDocumenter (confirmed it is actually executed by breaking it once).
🤖 Generated with Claude Code
https://claude.ai/code/session_01L8HenXTVrASo1sBZVGhpDQ