-
Notifications
You must be signed in to change notification settings - Fork 10
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
Solve systems with multiple rhs #81
Solve systems with multiple rhs #81
Conversation
Codecov ReportAll modified lines are covered by tests ✅
📢 Thoughts on this report? Let us know!. |
That's awesome! Thank you! I tried |
Is the norm of the difference between the solution with |
Ok the original PR gives accurate result after re-testing. However, I did a few tweaks to make it multi-threading, which surprisingly gives totally different result. See below: function lldl_lsolve!(n, X::AbstractMatrix, Lp, Li, Lx)
Threads.@threads for j = 1:n
@inbounds for p = Lp[j]:(Lp[j + 1] - 1)
@simd for k ∈ axes(X, 2)
X[Li[p], k] -= Lx[p] * X[j, k]
end
end
end
return X
end
function lldl_dsolve!(n, X::AbstractMatrix, D)
Threads.@threads for j = 1:n
@simd for k ∈ axes(X, 2)
X[j, k] /= D[j]
end
end
return X
end
function lldl_ltsolve!(n, X::AbstractMatrix, Lp, Li, Lx)
Threads.@threads for j = n:-1:1
@inbounds for p = Lp[j]:(Lp[j + 1] - 1)
@simd for k ∈ axes(X, 2)
X[j, k] -= conj(Lx[p]) * X[Li[p], k]
end
end
end
return X
end
function lldl_solve!(n, B::AbstractMatrix, Lp, Li, Lx, D, P)
@views Y = B[P, :]
lldl_lsolve!(n, Y, Lp, Li, Lx)
lldl_dsolve!(n, Y, D)
lldl_ltsolve!(n, Y, Lp, Li, Lx)
return B
end Any idea why it gives wrong result? |
I think I found the reason. |
Thanks @geoffroyleconte. There could also be methods for right-hand sides that are contiguous in memory, so that you can use |
@mzy2240 maybe you are modifying some elements in I'm not an expert in this subject, @amontoison do you know what could be wrong here? |
You should permute the for loops if you want to use But you do a complete backward and forward sweep of the columns of |
@geoffroyleconte Can we merge this PR? |
99829f3
to
48a7643
Compare
@geoffroyleconte Can you rebase your branch? |
48a7643
to
2df725d
Compare
Thanks @geoffroyleconte! |
Hello @mzy2240, does this fix your issue?
@dpo I copy-pasted some code that was in LDLFactorizations.jl for multiple rhs.