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

Let rdiv! accept a LU factorization object #31285

Merged
merged 9 commits into from
Apr 15, 2019

Conversation

zsoerenm
Copy link
Contributor

@zsoerenm zsoerenm commented Mar 7, 2019

The documentation of rdiv! implies that rdiv! does accept an factorization object. While this true for ldiv! it has not been implemented for rdiv!, yet.

  rdiv!(A, B)

  Compute A / B in-place and overwriting A to store the result.

  The argument B should not be a matrix. Rather, instead of matrices it should be a factorization object (e.g. produced by factorize or cholesky). The reason for this is that factorization itself is
  both expensive and typically allocates memory (although it can also be done in-place via, e.g., lu!), and performance-critical situations requiring rdiv! usually also require fine-grained control
  over the factorization of B.

This is an attempt to fix this at least for the lu factorization.

@kshyatt kshyatt added the linear algebra Linear algebra label Mar 7, 2019
@dkarrasch
Copy link
Member

Very nice work! Based on the swap changes in this PR, we could add support for lmul! and rmul! with LU objects.

function LinearAlgebra.lmul!(F::LU, B::AbstractMatrix)
    lmul!(UnitLowerTriangular(F.factors), lmul!(UpperTriangular(F.factors), B))
    _apply_inverse_ipiv_rows!(F, B)
end

function LinearAlgebra.rmul!(A::AbstractMatrix, F::LU)
    _apply_ipiv_cols!(F, A)
    rmul!(rmul!(A, UnitLowerTriangular(F.factors)), UpperTriangular(F.factors))
end

This might be helpful in cases of very large matrices...? For instance, if F was constructed from lu!(A), then A is no longer available to compute A * B or B * A. Moreover, the multiplication is inplace of B.

@carstenbauer
Copy link
Member

Since the changes have been approved, is there any chance for this to be in 1.2?

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

Successfully merging this pull request may close these issues.

5 participants