Skip to content

ENH: Add itk::Math LDLT symmetric solve/inverse and vnl_cholesky engine GTest#6565

Merged
hjmjohnson merged 3 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:enh-math-solve-symmetric-ldlt
Jul 10, 2026
Merged

ENH: Add itk::Math LDLT symmetric solve/inverse and vnl_cholesky engine GTest#6565
hjmjohnson merged 3 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:enh-math-solve-symmetric-ldlt

Conversation

@hjmjohnson

@hjmjohnson hjmjohnson commented Jul 8, 2026

Copy link
Copy Markdown
Member

Adds itk::Math::SolveSymmetric and itk::Math::InverseSymmetric, backed by Eigen's LDLT (Bunch-Kaufman) factorization, for symmetric (indefinite) linear systems and symmetric-matrix inversion. Also scavenges vxl test_cholesky.cxx into itkVnlCholeskyEngineGTest so the native Cholesky engine (98f7498) is covered in ITK CI, following the itkVnlSVDEngineGTest precedent (#6546). Split out of #6564.

API and rationale
  • SolveSymmetric(A, b) solves A x = b for symmetric A; a matrix-RHS overload solves A X = B with a single factorization.
  • InverseSymmetric(A) returns the symmetric inverse; it throws for singular input (LDLT pivot-diagonal check) rather than silently returning a pseudo-inverse.
  • Overloads take itk::Array2D / itk::Array and map the contiguous block through Eigen::Map with no copy.
  • ITK_MATH_HAS_SOLVE_SYMMETRIC lets consumers feature-test the header (#if __has_include(<itkMathLDLT.h>)).
  • LDLT is the right factorization for symmetric indefinite systems (covariance/precision matrices, normal equations): faster and more robust on ill-conditioned SPD inputs than a general solve.
Test coverage and legacy-state behavior
  • itkMathLDLTGTest: SPD and indefinite systems, matrix-RHS solve, symmetric inverse, singular-input rejection, vnl-oracle cross-checks (float and double).
  • itkVnlCholeskyEngineGTest (scavenged from vxl test_cholesky.cxx): inverse vs direct inverse, two-sided identity in default and estimate_condition modes, known-solution solve, determinant vs analytic, triangular-factor reconstruction, rank-deficiency/rcond, equivalence vs itk::Math::SolveSymmetricPositiveDefinite.
  • Both suites define ITK_LEGACY_TEST for their vnl oracles and compile those out under ITK_FUTURE_LEGACY_REMOVE, matching the vnl_cholesky deprecation guard already on main.
  • Verified locally in all three legacy states: default 20/20, ITK_LEGACY_REMOVE 20/20 (0 deprecation warnings), ITK_FUTURE_LEGACY_REMOVE 12/12 (deprecated-engine suites compiled out).

@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Enhancement Improvement of existing methods or implementation type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module labels Jul 8, 2026
@hjmjohnson hjmjohnson marked this pull request as ready for review July 8, 2026 22:34
@greptile-apps

This comment was marked as resolved.

Comment thread Modules/Core/Common/include/itkMathLDLT.h Outdated
@hjmjohnson hjmjohnson force-pushed the enh-math-solve-symmetric-ldlt branch 2 times, most recently from adbab66 to 4edeea4 Compare July 9, 2026 01:05
Add itk::Math::SolveSymmetric and itk::Math::InverseSymmetric for symmetric
(indefinite) linear systems and symmetric-matrix inversion, backed by Eigen's
LDLT (Bunch-Kaufman) factorization. Overloads accept itk::Array2D/itk::Array
and map the contiguous block through Eigen::Map with no copy; a matrix-RHS
overload solves A X = B with a single factorization. ITK_MATH_HAS_SOLVE_SYMMETRIC
advertises the capability so consumers can feature-test the header.

itkMathLDLTGTest checks SPD and indefinite systems, the matrix-RHS solve and
the symmetric inverse against a vnl reference.
@hjmjohnson hjmjohnson force-pushed the enh-math-solve-symmetric-ldlt branch from 4edeea4 to 29277be Compare July 9, 2026 01:11
Adapt vxl core/vnl/algo/tests/test_cholesky.cxx into an ITK GoogleTest so the
coverage runs in ITK CI and guards the native Cholesky engine that replaced
the netlib LINPACK path (98f7498). Beyond the scavenged cases (inverse vs
direct inverse, two-sided identity in both modes, known-solution solve), adds
determinant, triangular-factor, rank-deficiency/rcond, and an equivalence
check against the Eigen-backed itk::Math::SolveSymmetricPositiveDefinite.

The suite defines ITK_LEGACY_TEST and compiles out under
ITK_FUTURE_LEGACY_REMOVE, matching the vnl_cholesky deprecation guard.

Follows the itkVnlSVDEngineGTest precedent (PR InsightSoftwareConsortium#6546). Tracking: InsightSoftwareConsortium#6403.
@hjmjohnson hjmjohnson changed the title ENH: Add Eigen-backed itk::Math::SolveSymmetric and InverseSymmetric (LDLT) ENH: Add itk::Math LDLT symmetric solve/inverse and vnl_cholesky engine GTest Jul 9, 2026
@hjmjohnson hjmjohnson requested a review from thewtex July 10, 2026 18:21

@thewtex thewtex left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🎇 🥇

@hjmjohnson hjmjohnson merged commit 8defabd into InsightSoftwareConsortium:main Jul 10, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants