Skip to content

Commit

Permalink
Change the default QR to ColumnNorm
Browse files Browse the repository at this point in the history
  • Loading branch information
avik-pal committed Apr 18, 2024
1 parent d050e01 commit 4aef0e0
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 14 deletions.
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "LinearSolve"
uuid = "7ed4a6bd-45f5-4d41-b270-4a48e9bafcae"
authors = ["SciML"]
version = "2.28.0"
version = "2.29.0"

[deps]
ArrayInterface = "4fba245c-0d91-5ea0-9b3e-6abc04ee57a9"
Expand Down
1 change: 1 addition & 0 deletions src/LinearSolve.jl
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ needs_concrete_A(alg::AbstractKrylovSubspaceMethod) = false
needs_concrete_A(alg::AbstractSolveFunction) = false

# Util
## This is a check exclusively based on the size and not on the actual rank of the matrix
is_underdetermined(x) = false
is_underdetermined(A::AbstractMatrix) = size(A, 1) < size(A, 2)
is_underdetermined(A::AbstractSciMLOperator) = size(A, 1) < size(A, 2)
Expand Down
14 changes: 2 additions & 12 deletions src/default.jl
Original file line number Diff line number Diff line change
Expand Up @@ -216,19 +216,9 @@ function defaultalg(A, b, assump::OperatorAssumptions{Bool})
elseif assump.condition === OperatorCondition.WellConditioned
DefaultAlgorithmChoice.NormalCholeskyFactorization
elseif assump.condition === OperatorCondition.IllConditioned
if is_underdetermined(A)
# Underdetermined
DefaultAlgorithmChoice.QRFactorizationPivoted
else
DefaultAlgorithmChoice.QRFactorization
end
DefaultAlgorithmChoice.QRFactorizationPivoted
elseif assump.condition === OperatorCondition.VeryIllConditioned
if is_underdetermined(A)
# Underdetermined
DefaultAlgorithmChoice.QRFactorizationPivoted
else
DefaultAlgorithmChoice.QRFactorization
end
DefaultAlgorithmChoice.QRFactorizationPivoted

Check warning on line 221 in src/default.jl

View check run for this annotation

Codecov / codecov/patch

src/default.jl#L221

Added line #L221 was not covered by tests
elseif assump.condition === OperatorCondition.SuperIllConditioned
DefaultAlgorithmChoice.SVDFactorization
else
Expand Down
34 changes: 33 additions & 1 deletion test/default_algs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ solve(prob)

@test LinearSolve.defaultalg(nothing, zeros(5),
LinearSolve.OperatorAssumptions(false)).alg ===
LinearSolve.DefaultAlgorithmChoice.QRFactorization
LinearSolve.DefaultAlgorithmChoice.QRFactorizationPivoted

A = spzeros(2, 2)
# test that solving a singular problem doesn't error
Expand Down Expand Up @@ -112,3 +112,35 @@ prob = LinearProblem(funcop, b)
sol1 = solve(prob)
sol2 = solve(prob, LinearSolve.KrylovJL_CRAIGMR())
@test sol1.u == sol2.u

# Default for Underdetermined problem but the size is a long rectangle
A = [2.0 1.0
0.0 0.0
0.0 0.0]
b = [1.0, 0.0, 0.0]
prob = LinearProblem(A, b)
sol = solve(prob)

@test sol.u [0.4, 0.2]

## Show that we cannot select a default alg once by checking the rank, since it might change
## later in the cache
## Common occurence for iterative nonlinear solvers using linear solve

Check warning on line 128 in test/default_algs.jl

View workflow job for this annotation

GitHub Actions / Spell Check with Typos

"occurence" should be "occurrence".
A = [2.0 1.0
1.0 1.0
0.0 0.0]
b = [1.0, 1.0, 0.0]
prob = LinearProblem(A, b)

cache = init(prob)
sol = solve!(cache)

@test sol.u [0.0, 1.0]

cache.A = [2.0 1.0
0.0 0.0
0.0 0.0]

sol = solve!(cache)

@test sol.u [0.4, 0.2]

0 comments on commit 4aef0e0

Please sign in to comment.