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

RFC: Add julia ldlt factorization and solver for SymTridiagonal and julia lu factorization for Tridiagonal #6265

Merged
merged 1 commit into from
Apr 10, 2014

Conversation

andreasnoack
Copy link
Member

@alanedelman suggested Julia implementation for factorizations of SymTridiagonals. I have followed LAPACK and used ldlt for SymTridiagonal because it doesn't require allocation. I also wrote a solver for the factorization and removed the calls to LAPACK.

For Tridiagonal, I translated the lu from LAPACK to Julia since I didn't dare to open a pull request with a version without pivoting. To do so, I have changed the Tridiagonal type to only have one extra vector allocation. This is required for the pivoting. This also caused some cleanup of the tridiagonal code. I have changed all solve methods to A_ldiv_B to be consistent with the rest of Base. If anyone uses the Tridiagonal code please have a look and test.

A style innovation that I would like some comments on is that I have parametrized lu and ldlt on matrix type. In consequence there is only one lu type definition because it works for ordinary matrices as well as tridiagonals. This change also makes it possible to use array views in factorizations. I would like to extend this innovation to the other special matrices such that e.g. Triangular can be any AbstractMatrix.

Finally, I Implement convert(AbstractArray,x) and convert(Factorization,x) methods as proposed by @JeffBezanson in #6078. I get the same type of warnings could not show value of type Tuple. @JeffBezanson can you figure out what is wrong?

cc: @jiahao

@jiahao
Copy link
Member

jiahao commented Mar 25, 2014

I like the idea of parameterizing the factorization by matrix type. It would certainly clean up the documentation quite a bit too; the updated explanation of lufact was getting slightly out of hand.

I'm not sure it's necessary to deprecate solve everywhere as part of this PR. I'd prefer the scope of this PR to just deprecate solve for Tridiagonals and take the rest from there.

@timholy
Copy link
Sponsor Member

timholy commented Mar 25, 2014

For Tridiagonal, I translated the lu from LAPACK to Julia since I didn't dare to open a pull request with a version without pivoting.

😄

@andreasnoack
Copy link
Member Author

I'm not sure it's necessary to deprecate solve everywhere as part of this PR. I'd prefer the scope of this PR to just deprecate solve for Tridiagonals and take the rest from there.

I agree, but I think it makes sense to change the Woodbury solve also because it only worked with the solve(Tridiagonal). The solves in the sparse code can stay for now.

@andreasnoack
Copy link
Member Author

@JeffBezanson There is an issue with the ambiguity warnings beginning at line 840 here.

@alanedelman
Copy link
Contributor

Can we have pivoting=false as an option? Many use cases are symmetric positive definite matrices
where pivoting is not required for stability

@andreasnoack
Copy link
Member Author

Yes. I can add that option. It would also be useful to have that option for dense matrices as discussed [here](that option). Just curious, why do you prefer the lu over the ldlt?

… lu factorization for Tridiagonal. Parametrize LU on matrix type. Make pivoting optional in lu. Implement convert(AbstractArray,x) and convert(Factorization,x) methods.
andreasnoack added a commit that referenced this pull request Apr 10, 2014
Add julia ldlt factorization and solver for SymTridiagonal and julia lu factorization for Tridiagonal. Make pivoting optional in LU.
@andreasnoack andreasnoack merged commit 3e3ca28 into master Apr 10, 2014
@andreasnoack andreasnoack deleted the anj/symtri branch April 10, 2014 13:23
@andreasnoack
Copy link
Member Author

@alanedelman I have now merged the LDLt and LU for tridiagonal matrices and made pivoting optional, so you can write lufact(A,pivot=false).

@jiahao Unfortunately, I don't think it is possible to simplify the documentation for LU. I like the structure for the factorizations, by the way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants