Skip to content

Fix maxiter behavior for fallback gradient linear solver#261

Merged
leburgel merged 2 commits intomasterfrom
lb/maxiter_fix
Sep 23, 2025
Merged

Fix maxiter behavior for fallback gradient linear solver#261
leburgel merged 2 commits intomasterfrom
lb/maxiter_fix

Conversation

@leburgel
Copy link
Member

When falling back on a linear solver for computing the fixed-point gradient in case the eigsolver approach failed, the linear solver inherits its maxiter setting from the original eigsolver. However, due to difference in what an 'iteration' means in Krylov-based versus non-Krylov-based linear solvers from KrylovKit.jl, this default behavior can give a fallback solver with a very low maxiter setting.

In particular, now when using an Arnoldi solver with a small maxiter and a large krylovdim, the backup solver will be a BiCGStab solver with a small maxiter, while the krylovdim is just lost.

Ideally, KrylovKit.jl would allow some way of setting a maximum number of function applications (which is really what we want to do here) that works the same way across all algorithms regardless of whether or not they're actually Krylov methods. For now though, we can kind of emulate this behavior by just combining the maxiter and krylovdim of the eigsolver into the maxiter for the backup linear solver. I think this should be fine for now, and should at least avoid the issue until there is a better fix available.

@codecov
Copy link

codecov bot commented Sep 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
...rithms/optimization/fixed_point_differentiation.jl 94.48% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@lkdvos lkdvos left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me. In general we never really want to hit the maxiter anyways, and hope to reach the requested tolerance, so I think this is good.

@leburgel leburgel enabled auto-merge (squash) September 23, 2025 06:33
@leburgel leburgel disabled auto-merge September 23, 2025 10:38
@leburgel leburgel merged commit 999e37a into master Sep 23, 2025
50 of 51 checks passed
@leburgel leburgel deleted the lb/maxiter_fix branch September 23, 2025 10:39
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.

2 participants