Add documentation for all keyword arguments #682
Conversation
Codecov ReportBase: 98.24% // Head: 98.24% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #682 +/- ##
=======================================
Coverage 98.24% 98.24%
=======================================
Files 37 37
Lines 6599 6599
=======================================
Hits 6483 6483
Misses 116 116
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
src/minres.jl
Outdated
| resid_decrease_lim = (rNorm ≤ rNormtol) | ||
| # We must check that these stopping conditions work with preconditioners | ||
| # before we reuse them as stopping conditions. | ||
| # solved_lim = (test2 ≤ ε) |
There was a problem hiding this comment.
Hi, why is this removed? ratol and rrtol were added to be able to deactivate these criteria by setting atol and rtol to zero.
There was a problem hiding this comment.
Some clarifications about my Julia comment:
- I highly suspect that these stopping conditions are wrong when a preconditioner is provided;
- I need to verify that test1 and test2 are associated to backward errors and add the same tolerances as in LS/LN solvers (btol / axtol);
- MINRES is the only solver for square systems that uses these stopping conditions and the default tolerances must be tunned a little bit;
- the 32 other solvers use atol and rtol for stopping conditions based on the residual norm.
There was a problem hiding this comment.
I plan to add all stopping conditions in the documentation in the future and check one by one all these undocumented tests.
There was a problem hiding this comment.
I highly suspect that these stopping conditions are wrong when a preconditioner is provided;
Why would they be wrong? β₁ is the norm of the preconditioned residual. Those stopping tests are very useful when we know the system is consistent.
There was a problem hiding this comment.
test1 = rNorm / (ANorm * xNorm)
test2 = root / ANorm- test1:
rNormandANormare estimates based on the preconditioned linear systems whereasxNormis always an estimate of the solution of the initial linear systemAx = b. We computexNormwithxNorm = @knrm2(n, x). - test2: I think that we forgot to multiply by
ϕbarsuch thatϕbar * root / ANormrepresents‖Aᴴrₖ₋₁‖ / ‖A‖_F, buttest2could be also related to the backward error and I just don't understand the stopping condition.
There was a problem hiding this comment.
We'll have to look into that. Please open an issue but don't remove stopping tests.
There was a problem hiding this comment.
Is it ok to comment them the time that we find a fix?
It could do more harm than good to keep them broken.
There was a problem hiding this comment.
Let's not comment them out because we use them all the time (without a preconditioner). But let's look for a fix.
There was a problem hiding this comment.
Oh, It means that test2 is correct without a preconditioner? Can you explain what it represents?
I will rename these variables if I can, you're the only one that can understand them.
We could also use what I suggested more than one year ago: #279 (comment) for test1.
I will check the Matlab code of Mike before our next meeting to see how he handles the preconditioner case.
amontoison
changed the title
@paraynaud @tmigot @AntoninKns @geoffroyleconte