Skip to content

TRICG & TRIMR: support warm-start with ldiv=true#1043

Open
timholy wants to merge 6 commits intoJuliaSmoothOptimizers:mainfrom
timholy:teh/tricg_warmstart
Open

TRICG & TRIMR: support warm-start with ldiv=true#1043
timholy wants to merge 6 commits intoJuliaSmoothOptimizers:mainfrom
timholy:teh/tricg_warmstart

Conversation

@timholy
Copy link
Copy Markdown
Contributor

@timholy timholy commented Nov 25, 2025

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

NOTE: this is a WIP as trimr has not yet been modified. Once we settle on the tricg changes, I'll port the same changes to trimr.

@timholy timholy marked this pull request as draft November 30, 2025 19:53
@timholy
Copy link
Copy Markdown
Contributor Author

timholy commented Nov 30, 2025

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 trimr.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.64%. Comparing base (9536ef7) to head (3f3455b).
⚠️ Report is 122 commits behind head on main.

Files with missing lines Patch % Lines
src/trimr.jl 62.50% 6 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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`
@timholy timholy force-pushed the teh/tricg_warmstart branch from 2e863f9 to 3f3455b Compare February 22, 2026 23:20
Comment thread src/tricg.jl Outdated
Comment thread src/tricg.jl Outdated
@timholy timholy marked this pull request as ready for review February 24, 2026 09:39
Copilot AI review requested due to automatic review settings February 24, 2026 09:39
@timholy
Copy link
Copy Markdown
Contributor Author

timholy commented Feb 24, 2026

I also included a fix for #1076 for these two methods. Something similar may be needed for the others.

@timholy timholy force-pushed the teh/tricg_warmstart branch from 28f47ce to 00777df Compare February 24, 2026 09:44
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/F consistently for both ldiv=false and ldiv=true, and to restate residual norm weighting in terms of E/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=true with a non-identity N.

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.

Comment thread src/tricg.jl Outdated
Comment thread src/tricg.jl
Comment thread src/trimr.jl Outdated
Comment thread src/tricg.jl
Comment thread src/tricg.jl Outdated
Comment thread test/test_warm_start.jl
Comment thread src/trimr.jl
Comment thread src/trimr.jl
Comment thread src/tricg.jl Outdated
timholy and others added 2 commits February 24, 2026 03:59
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@timholy
Copy link
Copy Markdown
Contributor Author

timholy commented Feb 27, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants