From 5af3c2a8b42f50c585667b53f158fc4db6cc19e4 Mon Sep 17 00:00:00 2001 From: Andreas Noack Date: Fri, 23 Aug 2019 09:34:39 +0200 Subject: [PATCH] Use pivoting as the default in LU regardless of the element type. (#32989) For types that weren't subtypes of AbstractFloat, we used to try to LU factorize without pivoting and only use pivoting when it failed. This caused large numerical errors when computing the LU for element types which promoted to float like numbers such as most integers. The behavior was never documented and is error prone. Hence, this PR removes the behavior. --- stdlib/LinearAlgebra/src/lu.jl | 27 ++------------------------- stdlib/LinearAlgebra/test/lu.jl | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/stdlib/LinearAlgebra/src/lu.jl b/stdlib/LinearAlgebra/src/lu.jl index 00f0fa3253a75..107e5f6022a0a 100644 --- a/stdlib/LinearAlgebra/src/lu.jl +++ b/stdlib/LinearAlgebra/src/lu.jl @@ -179,13 +179,6 @@ function generic_lufact!(A::StridedMatrix{T}, ::Val{Pivot} = Val(true); return LU{T,typeof(A)}(A, ipiv, convert(BlasInt, info)) end -# floating point types doesn't have to be promoted for LU, but should default to pivoting -function lu(A::Union{AbstractMatrix{T}, AbstractMatrix{Complex{T}}}, - pivot::Union{Val{false}, Val{true}} = Val(true); - check::Bool = true) where {T<:AbstractFloat} - lu!(copy(A), pivot; check = check) -end - function lutype(T::Type) # In generic_lufact!, the elements of the lower part of the matrix are # obtained using the division of two matrix elements. Hence their type can @@ -274,26 +267,10 @@ julia> l == F.L && u == F.U && p == F.p true ``` """ -function lu(A::AbstractMatrix{T}, pivot::Union{Val{false}, Val{true}}; +function lu(A::AbstractMatrix{T}, pivot::Union{Val{false}, Val{true}}=Val(true); check::Bool = true) where T S = lutype(T) - AA = similar(A, S) - copyto!(AA, A) - lu!(AA, pivot; check = check) -end -# We can't assume an ordered field so we first try without pivoting -function lu(A::AbstractMatrix{T}; check::Bool = true) where T - S = lutype(T) - AA = similar(A, S) - copyto!(AA, A) - F = lu!(AA, Val(false); check = false) - if issuccess(F) - return F - else - AA = similar(A, S) - copyto!(AA, A) - return lu!(AA, Val(true); check = check) - end + lu!(copy_oftype(A, S), pivot; check = check) end lu(S::LU) = S diff --git a/stdlib/LinearAlgebra/test/lu.jl b/stdlib/LinearAlgebra/test/lu.jl index 24081d294762d..a4057fc7a8f7e 100644 --- a/stdlib/LinearAlgebra/test/lu.jl +++ b/stdlib/LinearAlgebra/test/lu.jl @@ -301,10 +301,19 @@ include("trickyarithmetic.jl") @testset "lu with type whose sum is another type" begin A = TrickyArithmetic.A[1 2; 3 4] ElT = TrickyArithmetic.D{TrickyArithmetic.C,TrickyArithmetic.C} - B = lu(A) + B = lu(A, Val(false)) @test B isa LinearAlgebra.LU{ElT,Matrix{ElT}} - C = lu(A, Val(false)) - @test C isa LinearAlgebra.LU{ElT,Matrix{ElT}} +end + +@testset "Issue #30917. Determinant of integer matrix" begin + @test det([1 1 0 0 1 0 0 0 + 1 0 1 0 0 1 0 0 + 1 0 0 1 0 0 1 0 + 0 1 1 1 0 0 0 0 + 0 1 0 0 0 0 1 1 + 0 0 1 0 1 0 0 1 + 0 0 0 1 1 1 0 0 + 0 0 0 0 1 1 0 1]) ≈ 6 end end # module TestLU