BiLQR & TriLQR #159
BiLQR & TriLQR #159
Conversation
amontoison
commented
Nov 14, 2019
•
- BiLQR and TriLQR methods for solving adjoint systems Ax=b and Aᵀt = c
- ODE and PDE adjoint problems used as tests
- Generalized TriLQR for rectangular matrices
- Allows inconsistent dual systems in TriLQR (solved by USYMQR)
- Stop primal or dual system when it's solved and continues the other one
- Improve verbose mode and statuses in both methods
- Use BiLQ, QMR, USYMLQ and USYMQR residual norm estimates described in the BiLQR / TriLQR article
Codecov Report
@@ Coverage Diff @@
## master #159 +/- ##
==========================================
+ Coverage 97.05% 97.43% +0.37%
==========================================
Files 26 28 +2
Lines 2176 2495 +319
==========================================
+ Hits 2112 2431 +319
Misses 64 64
Continue to review full report at Codecov.
|
0514b37 to
a50ec4f
Compare
a50ec4f to
7d5606a
Compare
|
The final home stretch before Krylov.jl v0.4.0 🎉 |
7d5606a to
f40ec71
Compare
f40ec71 to
35ac5e0
Compare
src/bilqr.jl
Outdated
| end | ||
|
|
||
| # Compute ⟨vₖ,vₖ₊₁⟩ and ‖vₖ₊₁‖ | ||
| vₖᵀvₖ₊₁ = @kdot(n, vₖ₋₁, vₖ) |
There was a problem hiding this comment.
Actually here you compute the dot product with vₖ₋₁, not vₖ₊₁. The comment may need to be adjusted too. Algorithm 2.2 in the paper has vₖᵀvₖ₊₁ on line 22.
There was a problem hiding this comment.
In BiLQ, vₖ is stored in vₖ₋₁ and vₖ₊₁ in vₖ it's why vₖᵀvₖ₊₁ = @kdot(n, vₖ₋₁, vₖ) but in BiLQR I moved the update of those vectors at the end to be able to stop primal / dual system before the other one and I forgot to update residual norm estimates...
There was a problem hiding this comment.
Ok. It's confusing that there's no variable vₖ₊₁.
Also, please don't mark comments as resolved. The reviewer is supposed to do that ;-).
There was a problem hiding this comment.
I will be able to use a variable vₖ₊₁ without allocations when y <- a * Ax + b * y operations will be added. I just need time to find a good way to implement them.
Ok, I won't mark comments resolved in the future :)
src/bilqr.jl
Outdated
| push!(sNorms, sNorm) | ||
|
|
||
| # Update dual stopping criterion | ||
| solved_dual = sNorm ≤ εQ || (sNorm + 1 ≤ 1) |
There was a problem hiding this comment.
Same comment here. This would allow you to set the final status based on which test triggered termination.
There was a problem hiding this comment.
For the final status, we should find a solution for #158. The number of combination of stopping criterion becomes problematic.
There was a problem hiding this comment.
Yes. For the time being, see, e.g., https://github.com/JuliaSmoothOptimizers/Krylov.jl/blob/master/src/lsqr.jl#L277.
There was a problem hiding this comment.
Thanks! I'm adding it.
|
Thank you! |