-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add julia lu factorization. Fix some promotion for linear systems and fix bidiagonal and triangular solvers for multiple right hand side. #5381
Conversation
Update:
|
The tests pass on both my Mac and Ubuntu. |
Maybe I should add a commercial for this pull request. With this you can do: inv([big(1)//(i+j-1) for i = 1:5,j=1:5])
5x5 Array{Rational{BigInt},2}:
25//1 -300//1 1050//1 -1400//1 630//1
-300//1 4800//1 -18900//1 26880//1 -12600//1
1050//1 -18900//1 79380//1 -117600//1 56700//1
-1400//1 26880//1 -117600//1 179200//1 -88200//1
630//1 -12600//1 56700//1 -88200//1 44100//1 |
Ah, brilliant. Likely to be quite slow, unfortunately, but still, very cool. |
woot!!!! |
|
The |
How about u2l in the name to indicate which way to goes. Otherwise I would never remember. |
Finally green. What about the |
Yes, let's discuss that separately. |
Okay, I think this one is ready to merge. I'll open a new issue for |
Thus will probably not be much slower than other libraries in cases like BigFloat, ehere much of the time will be spent in arithmetic. |
I have generalized the LU factorization to work for rectangular matrices similarly to what LAPACK does. @jiahao, do you have any further comments before merging? |
I'm guess that there is a missing int-to-float conversion in the linear solver test which is causing the InexactError in the test suite. |
This is weird. I cannot find the problem and the tests pass on all machines I have access to. As I can see, the error is in the infimum norm calculation in line 587, but why? |
The computation of julia> norm(convert(Matrix{Float16}, randn(5,5)), Inf)
ERROR: InexactError()
in setindex! at array.jl:342 The test fails because of this. |
Noup. It is the |
@jiahao Now I see what your comment was referring to. It was an intermediate error in my search for the real error. I have devectorized The tests are passing now, but the gcc machine timed out. |
I think we are good to go after the openlibm submodule hash is resolved. |
I would like to ask for some help on git here. I don't know why the change to the openlibm is included in the commit and I don't know how to get it out. |
… fix bidiagonal and triangular solvers for multiple right hand side. As consequence, change \ for bitarray to return Array{Float32} and introduce convert methods for sparse matrices without specifying integer type. Devectorize matrix norm.
Add julia lu factorization. Fix some promotion for linear systems and fix bidiagonal and triangular solvers for multiple right hand side.
🍰 |
Thanks. That was fast. |
It is crazy not to do pivoting, even for BigFloat, because not doing so can absurdly increase the required precision. |
This should not have been merged without pivoting, in my opinion. |
(This is only really useful for BigFloat, quaternions, and other user-defined fp types, in my opinion. Rational systems overflow too quickly unless you use BigInt, but with BigInt the complexity is generally exponential due to growth of the denominators, and exponential-complexity algorithms are practically useless. So I don't see the point of merging this until it can be taken seriously for fp types.) |
I agree that we would want a pivoted version so that down the line we could have something that is at least somewhat useful for |
I would go further and say that if the pivoted version isn't implemented soon, this patch should be reverted; we shouldn't include broken functionality, and LU without partial pivoting is broken. |
I have added a julia implementation of the LU decomposition and adjusted the higher level functions to call that when
eltype
is notBlasType
. The decomposition is stored in the same type as the LAPACK version. I don't think pivoting is so important wheneltype
is notBlasType
since the result should be exact for rational matrices and forMatrix{BigFloat}
you can always adjust the precision. If necessary, pivoting can be added later.The promotion fixes should also fix #5301
This is progress towards fixing #3014
I have changed the tests to use multiple right hand side to catch the error in the
Bidiagonal
/Triangular
solvers.