Skip to content

Commit

Permalink
Allow SparseArrays to catch lu(::WrappedSparseMatrix) (#51161)
Browse files Browse the repository at this point in the history
Over the `AbstractMatrix` relaxation in v1.9, we missed a potential
indirection for wrapped sparse matrices. Instead, by default, a
`similar` copy is created (hence a sparse matrix) and then `lu!` with a
pivot argument is called. Such a method, however, does not exist in
SparseArrays.jl, which means that the `generic_lufact!` method gets
called, which is probably really bad performance-wise, due to heavy
reading and writing into the sparse matrix. This PR introduces one more
level at which SparseArrays.jl (and perhaps other external packages) may
interfere and redirect to their own implementations, in-place or
out-of-place.
  • Loading branch information
dkarrasch authored Sep 5, 2023
1 parent 933ea1d commit 354c367
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions stdlib/LinearAlgebra/src/lu.jl
Original file line number Diff line number Diff line change
Expand Up @@ -296,12 +296,13 @@ julia> l == F.L && u == F.U && p == F.p
true
```
"""
function lu(A::AbstractMatrix{T}, pivot::Union{RowMaximum,NoPivot,RowNonZero} = lupivottype(T); check::Bool = true) where {T}
lu!(_lucopy(A, lutype(T)), pivot; check = check)
end
lu(A::AbstractMatrix{T}, args...; kwargs...) where {T} =
_lu(_lucopy(A, lutype(T)), args...; kwargs...)
# TODO: remove for Julia v2.0
@deprecate lu(A::AbstractMatrix, ::Val{true}; check::Bool = true) lu(A, RowMaximum(); check=check)
@deprecate lu(A::AbstractMatrix, ::Val{false}; check::Bool = true) lu(A, NoPivot(); check=check)
# allow packages like SparseArrays.jl to interfere here and call their own `lu`
_lu(A::AbstractMatrix, args...; kwargs...) = lu!(A, args...; kwargs...)

_lucopy(A::AbstractMatrix, T) = copy_similar(A, T)
_lucopy(A::HermOrSym, T) = copymutable_oftype(A, T)
Expand Down

2 comments on commit 354c367

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Please sign in to comment.