Skip to content

Fix OptimizationManopt reporting MaxIters even when converged - #1258

Draft
SebastianM-C wants to merge 1 commit into
masterfrom
smc/manopt-maxiters
Draft

Fix OptimizationManopt reporting MaxIters even when converged#1258
SebastianM-C wants to merge 1 commit into
masterfrom
smc/manopt-maxiters

Conversation

@SebastianM-C

Copy link
Copy Markdown
Member

Summary

  • __map_optimizer_args! fully replaced Manopt's own per-solver default stopping_criterion with just StopAfterIteration/StopAfter whenever maxiters/maxtime was given without an explicit abstol. Since those criteria never indicate convergence (Manopt's has_converged semantics), this made ReturnCode.MaxIters structurally unavoidable regardless of whether the solve had actually converged. Fixes the underlying issue behind Improve return codes from OptimizationManopt #1034, which Improve return codes from OptimizationManopt #1169's retcode-mapping fix exposed but never actually resolved.
  • Added a per-optimizer fallback tolerance (_default_fallback_abstol), used only when maxiters/maxtime is set without abstol, mirroring each solver's own real Manopt default so genuine convergence is still reported as Success. Optimizers whose real default is a composite criterion (NelderMead, CMAES, ConvexBundle) opt out and keep prior behavior.
  • Fixed the Karcher-mean docs example, which claimed to use Frank-Wolfe with a closed-form subsolver but never reassigned opt from the earlier GradientDescentOptimizer — Manopt silently warned and dropped the unrecognized sub_problem kwarg, so it silently ran plain gradient descent instead.
  • Adjusted for the Manopt 0.6 sub_problem calling-convention change (now needs evaluation = Manopt.InplaceEvaluation() for the mutating closed-form style) and bumped maxiters budgets across all three docs examples to values confirmed sufficient for them to reach Success.
  • Verified with Manopt.check_gradient that the two examples without an explicit abstol (Rosenbrock/Euclidean and Rayleigh quotient/Sphere) have correct gradients — their MaxIters was genuinely insufficient default iteration budget, not a gradient bug.

Test plan

  • Pkg.test() in lib/OptimizationManopt — all 16 tests pass, including the pre-existing Quasi-Newton/ARC/TrustRegions tests
  • Added a regression test asserting successful_retcode for a maxiters-only (no abstol) gradient-descent run
  • Ran all three docs/src/optimization_packages/manopt.md examples end-to-end; each now reports retcode = Success with the expected minimizer/objective

🤖 Generated with Claude Code

Passing `maxiters`/`maxtime` without an explicit `abstol` fully replaced
Manopt's own per-solver default stopping criterion with just
StopAfterIteration/StopAfter, which never indicate convergence per
Manopt's has_converged semantics. That made ReturnCode.MaxIters
structurally unavoidable regardless of whether the solve had actually
converged (see #1034, which the retcode-mapping fix in #1169 exposed
but never actually resolved).

Add a per-optimizer fallback tolerance, used only when maxiters/maxtime
is set without abstol, mirroring each solver's own real Manopt default
so genuine convergence can still be reported as Success. Also fix the
Karcher-mean docs example, which never actually used the Frank-Wolfe
optimizer or closed-form subsolver it claims to (opt was never
reassigned from the earlier GradientDescentOptimizer, so Manopt
silently dropped the unrecognized sub_problem kwarg), and adjust the
Manopt 0.6 sub_problem calling convention (InplaceEvaluation) plus
maxiters budgets across all three docs examples so each one now
reports Success.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

1 participant