Skip to content

Change default fixed-point gradient algorithm to :eigsolver#246

Merged
leburgel merged 4 commits intomasterfrom
lb/arnoldi_fallback
Aug 26, 2025
Merged

Change default fixed-point gradient algorithm to :eigsolver#246
leburgel merged 4 commits intomasterfrom
lb/arnoldi_fallback

Conversation

@leburgel
Copy link
Member

Changes the default fixed-point gradient algorithm to :eigsolver, and adds a fallback linsolve in case the eigsolver approach fails.

@leburgel leburgel marked this pull request as draft August 21, 2025 13:03
@codecov
Copy link

codecov bot commented Aug 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

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

@leburgel leburgel changed the title [WIP] Change default fixed-point gradient algorithm to :eigsolver Change default fixed-point gradient algorithm to :eigsolver Aug 22, 2025
@leburgel leburgel marked this pull request as ready for review August 22, 2025 09:36
@leburgel leburgel requested review from lkdvos and pbrehmer August 22, 2025 09:38
Comment on lines +396 to +400
backup_ls_alg = GMRES(; tol = alg.solver_alg.tol, maxiter = alg.solver_alg.maxiter, verbosity = alg.solver_alg.verbosity)
y, info = reallinsolve(∂f∂x, ∂F∂x, x₀, backup_ls_alg, 1, -1)
if alg.solver_alg.verbosity > 0 && info.converged != 1
@warn("backup gradient fixed-point iteration reached maximal number of iterations:", info)
end
Copy link
Member

Choose a reason for hiding this comment

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

How do you feel about making a backup alg which is LinSolver, and just returning fpgrad(args..., alg::LinSolver)? It removes a tiny bit of code, but it might also be slightly more readable.

leburgel and others added 2 commits August 26, 2025 09:52
Co-authored-by: Paul Brehmer <paul.brehmer@univie.ac.at>
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 good for me once tests pass

@leburgel leburgel enabled auto-merge (squash) August 26, 2025 10:25
@leburgel leburgel merged commit 1a27f11 into master Aug 26, 2025
43 of 45 checks passed
@leburgel leburgel deleted the lb/arnoldi_fallback branch August 26, 2025 13:07
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