Account for pivoting in sparse Cholesky decompositions #175
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.
As explained in the docstring, one should be careful when working with Cholesky decompositions of sparse arrays since the property
:Ldoes not take into account the pivoting - in contrast to:PtLwhich also includes the necessary permutations.This PR changes
chol_lowerandchol_upperfor such sparse Cholesky decompositions such that pivoting is taken into account. Unfortunately, since SparseArrays does not support multiplication of suchFactorComponents, code that multiplies the output ofchol_lower/chol_upperrequires a different, explicitly constructed sparse equivalent of these factors. Alternatively, one could define internal functions for multiplying the output ofchol_lower/chol_upper, let them fall back to*by default, and add special implementations for these sparse factors. I've not done this for now because I was not completely sure whether the sparse special case justifies such changes to the (internal) API.Additionally, the PR adds a seemingly missing definition of
chol_upper(::Matrix)and fixes a few test errors in test/pdmtypes.jl and test/specialarrays.jl caused by upstream bugs.Fixes #120.
Edit: I suggest merging #180 first. That PR fixes the test issues on the master branch properly whereas this PR here only contains a workaround.