Improve return codes from OptimizationManopt#1169
Conversation
|
Thanks, this looks like a good start, I did not find the time to follow up after the last mentioned issue, but something along your lines here is what I had in mind. I am a bit surprised there are so many tests failing, but even on master several tests fail, so maybe that is expected? |
|
It's just the two right? Minibatching, which I'll put a fix in, and something happened to PythonCall installation? |
|
I see 6 failing tests, but there are a bit too many as that I can see what it actually failing, since I am not even able to see a single of the stack trace lines on a single screen. |
|
https://github.com/SciML/Optimization.jl/actions/runs/23171032225/job/67322714966#step:7:1 has the tests for the Manopt integration and we have only 14 tests that all pass. All the other test groups are unrelated to Manopt (as far as I can tell) |
|
There are so many test CIs that I missed that, that looks indeed good to me together with the code I see here changed :) As far as I see then also the examples in the docs (when run later on master) should be fine (cf. https://github.com/SciML/Optimization.jl/blob/master/docs/src/optimization_packages/manopt.md) |
- Add default convergence-indicating stopping criteria: - Gradient-based solvers: StopWhenGradientNormLess (default 1e-8) - Derivative-free solvers: StopWhenChangeLess (default 1e-8) - Map abstol to the appropriate convergence criterion per solver type - Return ReturnCode.MaxIters when StopAfterIteration is the active stopping reason, instead of ReturnCode.Failure - Return ReturnCode.Success when Manopt.has_converged reports true Fixes #1034 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Map additional Manopt stopping criteria to specific SciML return codes: - StopAfter (time limit) → ReturnCode.MaxTime - StopWhenCostNaN/StopWhenIterateNaN → ReturnCode.Unstable - StopWhenStepsizeLess → ReturnCode.Stalled Also fix StopAfterTime → StopAfter (correct Manopt type name). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Don't inject a default 1e-8 convergence criterion — let Manopt's own per-solver defaults handle stopping. Each solver has tailored defaults (e.g., GD uses StopWhenGradientNormLess(1e-8), ParticleSwarm uses StopWhenChangeLess(1e-4), CMA-ES has multiple specialized criteria). When no stopping criteria are specified at all, don't pass stopping_criterion to the solver, letting Manopt use its built-in defaults entirely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These were marked broken because retcodes always returned Failure. Now that we properly use Manopt.has_converged, they correctly return Success. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SebastianM-C
force-pushed
the
smc/manopt-retcodes
branch
from
509f916 to
14578b1
Compare
|
@ChrisRackauckas is this good to merge? I rebased it on the latest master |
3 participants
Fix the convergence criteria
Map additional Manopt stopping criteria to specific SciML return codes:
Fixes #1034
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add any other context about the problem here.