TRICG & TRIMR: support warm-start with ldiv=true#1043
TRICG & TRIMR: support warm-start with ldiv=true#1043timholy wants to merge 6 commits intoJuliaSmoothOptimizers:mainfrom
Conversation
|
To clarify, this is ready for review despite me having switched it to "draft." I did that just because it shouldn't be merged until I make commensurate changes to |
f5ea9af to
4683dad
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1043 +/- ##
==========================================
+ Coverage 94.68% 97.64% +2.96%
==========================================
Files 45 50 +5
Lines 8027 10027 +2000
==========================================
+ Hits 7600 9791 +2191
+ Misses 427 236 -191 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This addresses some potential inconsistencies in `tricg` and `trimr` in the case `ldiv=true`: - docs: fixes the statement about the relationship between `E`/`F` and `M`/`N` - docs: describes the residual-norm in terms of `E`/`F` rather than `M`/`N` (only the former are unambiguous, given the problem statement) - adds support for warm-starting when `ldiv=true` for arbitrary `M`/`N`
2e863f9 to
3f3455b
Compare
|
I also included a fix for #1076 for these two methods. Something similar may be needed for the others. |
28f47ce to
00777df
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the TriCG/TriMR saddle-point solvers to better support warm-starting when ldiv=true, and clarifies the documentation around how the weighting operators E/F relate to the user-provided preconditioners M/N.
Changes:
- TriCG docstring updated to describe
E/Fconsistently for bothldiv=falseandldiv=true, and to restate residual norm weighting in terms ofE/F. - TriCG and TriMR relax the “warm-start with preconditioners not supported” restriction when
ldiv=true, and rework initialization so warm-start can reinitialize with the shifted RHS. - Added a TriCG warm-start regression test for
ldiv=truewith a non-identityN.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
src/tricg.jl |
Doc clarifications for E/F vs M/N; enables warm-start path when ldiv=true; adjusts initialization sequence. |
src/trimr.jl |
Mirrors the warm-start gating/initialization changes for ldiv=true (despite PR note calling TrimR “not yet modified”). |
test/test_warm_start.jl |
Adds a warm-start round-trip test for tricg(...; N, ldiv=true) to validate the new behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
I believe this is ready for merging. Since it is about warm-starting, I also went ahead and implemented a fix for #1076 for these two methods. |
This addresses some potential inconsistencies in
tricgandtrimrinthe case
ldiv=true:E/FandM/NE/Frather thanM/N(only the former are unambiguous, given the problem statement)ldiv=truefor arbitraryM/NNOTE: this is a WIP astrimrhas not yet been modified. Once we settle on thetricgchanges, I'll port the same changes totrimr.