Skip to content

GMRES sets success flag to true if the initial guess is already converged#574

Open
michelebucelli wants to merge 2 commits into
SimVascular:mainfrom
michelebucelli:fix/gmres-warning
Open

GMRES sets success flag to true if the initial guess is already converged#574
michelebucelli wants to merge 2 commits into
SimVascular:mainfrom
michelebucelli:fix/gmres-warning

Conversation

@michelebucelli

@michelebucelli michelebucelli commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Fixes #573:

  1. the success flag is now set to true in GMRES if the initial guess is already good enough, so that no "solution has not converged" warnings are raised;
  2. the success flag itself is renamed from suc to success for improved readability.

Code of Conduct & Contributing Guidelines

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@michelebucelli

michelebucelli commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Partially related to this: I asked AI to double-check that I'd caught all instances of suc to be renamed to success (I don't have PETSc on my local build so I might have missed some), and it observed that there's a solver/gmres.cpp file that is nearly identical to linear_solver/gmres.cpp, but is never compiled (it's not listed in solver/CMakeLists.txt).

Perhaps it is a leftover from an older organization of the code? In which case, it might be worth removing it to clean it up, as part of this PR.

It also flagged a number of similarly dead code in the class lsType from ComMod.h.

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.72727% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 69.45%. Comparing base (9632608) to head (7ee150a).

Files with missing lines Patch % Lines
Code/Source/solver/output.cpp 85.71% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #574   +/-   ##
=======================================
  Coverage   69.44%   69.45%           
=======================================
  Files         181      181           
  Lines       34072    34077    +5     
  Branches     5930     5930           
=======================================
+ Hits        23662    23667    +5     
  Misses      10273    10273           
  Partials      137      137           

☔ View full report in Codecov by Harness.
📢 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.

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.

Suppress warning "The linear system solution has not converged" if no linear solver iterations are needed

1 participant