update Manopt and associated packages #906
Conversation
|
The changes are listed extensively, but the main change is that a lot of parts of Manopt.jl became easier to use with the new factory pattern introduced in Manopt.jl@0.5.0 – all renaming are mainly because the old “visible/public” type is now more an internal one and the new “visible/public” one is easier to use since it no longer requires the manifold. Usually if no manifold is given that means a factory is provided (storing mainly all keywords that were provided). Then the factory “fills in” defaults as soon as a manifold is provided. For the PR here I can try to find some time to look into and help, but I am literally just these days “falling out” of a semester with the last administrative duties. |
| @@ -67,7 +67,7 @@ function call_manopt_optimizer( | |||
| x0; | |||
| stopping_criterion::Union{Manopt.StoppingCriterion, Manopt.StoppingCriterionSet}, | |||
| evaluation::AbstractEvaluationType = Manopt.AllocatingEvaluation(), | |||
| stepsize::Stepsize = ArmijoLinesearch(M), | |||
| stepsize::Stepsize = ArmijoLinesearchStepsize(M), | |||
There was a problem hiding this comment.
This can for example stay as before, as I just wrote in the general comment ArmijoLiineSearch now introduces a factory (ok it is not a Stepsise type then, and when necessary one can produce_type on that, see the pattern in action at
(note the new union there)
as well as
which “turns” a factory into the concrete thing (here the stepwise) and is the identity if one already provides a stepwise.
| @@ -33,7 +33,7 @@ end | |||
| x0 = zeros(2) | |||
| p = [1.0, 100.0] | |||
|
|
|||
| stepsize = Manopt.ArmijoLinesearch(R2) | |||
| stepsize = Manopt.ArmijoLinesearch() | |||
There was a problem hiding this comment.
Ah note that this for example is still “just” a factory when written this way, so maybe an OptimizationProblem should use our internal function _produce_type or maybe better only rely on the concrete step size types and not the factories.
I hope this does not sound too chaotic, as the one having designed it, to me the scheme is relatively clear, and I hope I am able to explain it well enough.
There was a problem hiding this comment.
yeah. I think what we probably want to do is hook into the init and resolve the factory at that time? @ChrisRackauckas what do you think?
|
It would also be nice to check whether
|
|
Just because I saw that, the name of the package is “Manopt” with a small o, since we follow the capitalisation of the first one of the overall family, the Matlab one, though the Julia one is a complete redesign (more Julian).. |
This doesn't work yet, but @ChrisRackauckas or @Vaibhavdixit02 can one of you help get this updated? This is blocking support for ForwardDiff@1. https://github.com/JuliaManifolds/Manopt.jl/blob/master/Changelog.md shows some pretty extensive changes.