Skip to content

Add cholesky overload for tracer matrices#322

Merged
adrhill merged 1 commit into
adrhill:mainfrom
devmotion:cholesky-overload
Jun 24, 2026
Merged

Add cholesky overload for tracer matrices#322
adrhill merged 1 commit into
adrhill:mainfrom
devmotion:cholesky-overload

Conversation

@devmotion

Copy link
Copy Markdown
Contributor

I wanted to use the global tracer on a function that calls cholesky, and it errored: the pivot / positive-definiteness checks compare matrix entries, which on tracers return a tracer rather than a Bool.

This adds a cholesky overload mirroring the existing eigen overload — collapse the input to one conservative tracer via second_order_or and broadcast it over the factor with a lazy Fill. Both the default / NoPivot (→ Cholesky) and RowMaximum (→ CholeskyPivoted) variants are covered; the pattern is independent of pivoting.

Reproducer (errors on main, works here):

using SparseConnectivityTracer, LinearAlgebra
detector = TracerSparsityDetector()
f(x) = (A = reshape(x, 2, 2); C = cholesky(A' * A + 4I); sum(C.factors))
jacobian_sparsity(f, rand(4), detector)   # 1×4 all-ones after the fix

Adds a "Cholesky" testset mirroring "Eigenvalues".


This pull request was written with the assistance of generative AI (Claude Code).

A generic `cholesky` performs pivot / positive-definiteness checks that
compare matrix entries; on tracers those comparisons return a tracer
instead of a `Bool`, so the global tracer cannot factorize a matrix (e.g.
while constructing a dense `MvNormal`/`PDMat`). Mirror the existing `eigen`
overload: collapse the input to a single conservative tracer and broadcast
it over the factor with a lazy `Fill`. Both the default/`NoPivot` and the
`RowMaximum` variants are covered, since the all-depends pattern does not
depend on the pivoting strategy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread src/overloads/arrays.jl
LinearAlgebra.checksquare(A)
n = size(A, 1)
t = second_order_or(A)
return LinearAlgebra.Cholesky(Fill(t, n, n), 'U', 0)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

For future reference (either here or a comment in the code):
What are the extra arguments to Cholesky?

@devmotion devmotion Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The first argument is the upper or lower-triangular factor, the second argument indicates whether it's the upper ('U') or lower ('L') triangular factor, and the third argument is the BLAS info code (0 indicates success).

Comment thread src/overloads/arrays.jl
LinearAlgebra.checksquare(A)
n = size(A, 1)
t = second_order_or(A)
return LinearAlgebra.CholeskyPivoted(Fill(t, n, n), 'U', 1:n, n, tol, 0)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

For future reference (either here or a comment in the code):
What are the extra arguments to CholeskyPivoted?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The first two arguments and the last argument are the same as for Cholesky (factor, type of factor and BLAS info code), the third argument is the pivoting, and the fourth argument the rank.

@codecov-commenter

codecov-commenter commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.81%. Comparing base (52b6612) to head (de12e36).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #322      +/-   ##
==========================================
+ Coverage   93.76%   93.81%   +0.05%     
==========================================
  Files          28       28              
  Lines        1090     1100      +10     
==========================================
+ Hits         1022     1032      +10     
  Misses         68       68              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@adrhill

adrhill commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Thanks!

@adrhill adrhill merged commit c990469 into adrhill:main Jun 24, 2026
10 checks passed
@devmotion

Copy link
Copy Markdown
Contributor Author

Great, could you make a new release with the change?

@devmotion devmotion deleted the cholesky-overload branch June 24, 2026 09:02
@adrhill

adrhill commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Done, note that I tagged 1.2.2 instead of 1.3.0 pre-coffee.

@devmotion

Copy link
Copy Markdown
Contributor Author

No worries, that's fine with me 🙂

@devmotion

Copy link
Copy Markdown
Contributor Author

Thank you!

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.

3 participants