Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added choleskyinv.jl unit #5

Merged
merged 15 commits into from
Feb 8, 2020
Merged

Added choleskyinv.jl unit #5

merged 15 commits into from
Feb 8, 2020

Conversation

Marco-Congedo
Copy link
Contributor

In some situations the Cholesky factor and its inverse are both needed. This unit implements a Gaussian elimination recursion to compute the Cholesky factor and its inverse in one pass. On small matrices at the moment being this appears faster then using the standard Julia cholesky function and inverting the result. The main function creates two Cholesky factorizations, thus they inherit all special methods from LinearAlgebra.

I am having two problems at this time :

  1. I am not able to create a Cholesky factor with an UpperTriangular matrix as input, and since the algorithm computes the inverse of the Cholesky factor (complex-conjugate) transpose, there is a waste of time at the end to transpose the result.
  2. For the show method i would like to show the two factorizations calling the show method of LinearAlgebra, but i am currently not succeding.

Also, if anybody knows more efficient way to initialize to the identity matrix the LowerTriangular and UpperTriangular matrices used by the algorithms, please let me know.

This unit implements a Gaussian elimination recursion to compute the Cholesky factorization and its inverse in one pass. For small matrices this is faster then using `cholesky` and inversing the Cholesky factor with LinearAlgebra.jl. The main function creates two Cholesky factorizations, inheriting all the methods from LinearAlgebra.
@codecov
Copy link

codecov bot commented Feb 4, 2020

Codecov Report

Merging #5 into master will increase coverage by 0.10%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #5      +/-   ##
==========================================
+ Coverage   83.63%   83.73%   +0.10%     
==========================================
  Files           3        4       +1     
  Lines         385      412      +27     
==========================================
+ Hits          322      345      +23     
- Misses         63       67       +4     
Impacted Files Coverage Δ
src/ql.jl 83.41% <0.00%> (-0.53%) ⬇️
src/choleskyinv.jl 93.75% <0.00%> (ø)
src/qr.jl 83.16% <0.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c95f04...1f87a97. Read the comment docs.

src/choleskyinv.jl Outdated Show resolved Hide resolved
src/choleskyinv.jl Outdated Show resolved Hide resolved
src/choleskyinv.jl Outdated Show resolved Hide resolved
src/choleskyinv.jl Outdated Show resolved Hide resolved
src/choleskyinv.jl Outdated Show resolved Hide resolved
@Marco-Congedo
Copy link
Contributor Author

Hello, i made all the changes. How should i proceed now?

@dlfivefifty
Copy link
Member

The tests are failing. Can you fix them?

@Marco-Congedo
Copy link
Contributor Author

On my computer the testing are all passing. The only thing didn't work was the iterate, so i removed it for the moment being.

@Marco-Congedo
Copy link
Contributor Author

I think i know where the problem comes from. I m trying to fix it.

@Marco-Congedo
Copy link
Contributor Author

Hello, now it seems to work on Julia > 1.2. I am calling require_one_based_indexing and checksquare from LinearAlgebra to be in line with Julia's cholesky methods. Maybe those are not supported in Julia 1.0 and 1.1?

https://travis-ci.org/JuliaMatrices/MatrixFactorizations.jl/builds/647313386?utm_source=github_status&utm_medium=notification

@Marco-Congedo
Copy link
Contributor Author

@dlfivefifty
Copy link
Member

We can drop support for Julia v1.0 and 1.1. Modify Project.toml (and bump to v0.2)

@Marco-Congedo
Copy link
Contributor Author

In Project.Toml it is v0.2.1 now. Do you mean 0.3 or 0.2.2?

@dlfivefifty
Copy link
Member

Has to be 0.3 since we are dropping support for some Julia versions

@Marco-Congedo
Copy link
Contributor Author

Right, let me optimize the code a bit further before doing that. Also, what do you think to modify the function to check the dimansion of the matrices; if they are small go ahead with the closkyinv function, otherwise call Julia's standard cholesky and inv functions? This would provide a general function that would be usable in general.

Now it does not require any extra memory:
- the ``L_1`` factor overwrites the strictly lower triangle of the input matrix
- its complex-conjugate transpose inverse overwrites the stricly upper triangle of the input matrix
- ``D`` overwrites the diagonal part of the input matrix
@Marco-Congedo
Copy link
Contributor Author

Hello, Travis build runs smoothly now. For codecov, i have never used it, so, i am not sure what to do with the failing check.

@dlfivefifty dlfivefifty merged commit dcb647d into JuliaLinearAlgebra:master Feb 8, 2020
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