Improve return codes from OptimizationManopt

[ChrisRackauckas]

In #1009 it was mentioned that the return codes from OptimizationManopt always return failure. This is because https://github.com/SciML/Optimization.jl/blob/v4.8.0/lib/OptimizationManopt/src/OptimizationManopt.jl#L460 it's overridden so that it's always Success or Failure. I am not sure why Manopt.indicates_convergence(asc) is always false but that would be the reason for seeing the Failure outputs. My guess is that the comment isn't supposed to mean "it always returns failure" but instead is maybe something more specific, like "it always returns failure on this specific example"? @kellertuer it would help if you could clarify this.

My guess as to where the disconnect comes from is that:

prob = OptimizationProblem(optf, U; manifold = M, maxiters = 1000)

sol = Optimization.solve(
    prob, opt, sub_problem = (M, q, p, X) -> closed_form_solution!(M, q, L, U, p, X))

This solves for 1000 iterations, and then when we check Manopt.indicates_convergence(asc) that says it did not converge in 1000 iterations. The right thing to do here may be to override that convergence check if the exit condition is due to hitting maximum iterations.

@kellertuer are all of the examples you encountered that you're talking about similar? Because if so then it seems like that has an easy fix. Otherwise I'll need more information as to what you're referring to.

[kellertuer]

What I mean is if you look at the page

https://docs.sciml.ai/Optimization/stable/optimization_packages/manopt/

Every example run ends with “retcode: Failure”. That might convey to a reader “Manopt never works”. I do not like that.

I have not yet understood the retcodes here really and not yet found their documentation.
I think before the linked PR they really just always returned Failure, not matter what actually happened.

Concerning the technical details, the
https://manoptjl.org/stable/plans/stopping_criteria/#Manopt.indicates_convergence-Tuple%7BStoppingCriterion%7D
was a function I specifically wrote to address this. It does the following:
Every stopping criterion itself “knows” whether it can produce a certificate of convergence.

Example:

• s1 = StopAfterIteration(1000) (others might call this maxiter) never would produce that since after 1000 Iterations you can still be anywhere (not even close to a minimiser) depending on how you wrote your problem and how good your gradient for example is (e.g. due to numerical instabilities9
• s2 = StopWhenGradientNormLess(1e-10) does indicate convergence, since we are close to a critical point then.
• s1 & s2 indicates convergence
• s1 | s2 only indicates convergence if s2 is the reason this indicated to stop.

To me it is unclear what you mean with “override this”. There is nothing I can override? This is what mathematically has to be checked. I can not override math here.

I do assume the current examples are all wrong and contain errors.
But I could not yet run them on the PR – which was now merged in just a few hours after “the docs should run now”, before I even had the chance to notice that the docs should run now.

Dos that clarify the thing? Someone now has to look at the examples whether they are in the s1 scenarios – and then probably fix the gradient or so – or what is going on with them. But that is a matter of the examples.

I would prefer if that is addressed because – as I said – until then the documentation here indicated “Manopt never works”, which I am not so much a fan of if that is a messages conveyed.

[kellertuer]

Oh, sorry, I mixed something up above. There is (the older)
https://manoptjl.org/stable/plans/stopping_criteria/#Manopt.indicates_convergence-Tuple%7BStoppingCriterion%7D

which does the static analysis – so only based on the static types (or generically stopping criteria involved). In other words for s1 & s2 above it would return false, since that does not always indicate that an algorithm converged, it would return true for s2 and hence also for s1 & s2.

The new function I specifically wrote for the code here is
https://manoptjl.org/stable/plans/stopping_criteria/#Manopt.has_converged-Tuple%7BStoppingCriterion%7D

That does the analysis dynamically, or in the example for s1 | s2 it would look, which of the criteria is active during a solver run and indicate from there whether it has converged.
So for this example, if it hits maxiter (but not at the same time as well a small gradient) – it would indicate it has not converged.
The other way around if s2 indicated to stop, it would indicate it has converged.

And yes has_converged I wrote just to address issues in this package. If indicates_convergene is still used somewhere it should be replaced.

Sorry for the mixup (in CEST is it still a bit early – before first coffee).