Skip to content

update Manopt and associated packages #906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

update Manopt and associated packages #906

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88e06da
update ManOpt and associated packages
oscardssmith Apr 29, 2025
daa0b2e
Update stepsize kwargs to use the default constructor
Vaibhavdixit02 Apr 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lib/OptimizationManopt/Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ Reexport = "189a3867-3050-52da-a836-e630ba90ab69"

[compat]
LinearAlgebra = "1.10"
ManifoldDiff = "0.3.10"
Manifolds = "0.9.18"
ManifoldsBase = "0.15.10"
Manopt = "0.4.63"
ManifoldDiff = "0.3.10, 0.4"
Manifolds = "0.9.18, 0.10"
ManifoldsBase = "0.15.10, 1"
Manopt = "0.5"
Optimization = "4"
Reexport = "1.2"
julia = "1.9"
Expand Down
6 changes: 3 additions & 3 deletions lib/OptimizationManopt/src/OptimizationManopt.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Copy link
Contributor

@kellertuer kellertuer Apr 29, 2025

Choose a reason for hiding this comment

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

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

https://github.com/JuliaManifolds/Manopt.jl/blob/013feadf95736639cdedb1bff2c4f1723f7dfe64/src/solvers/gradient_descent.jl#L208-L210

(note the new union there)

as well as

https://github.com/JuliaManifolds/Manopt.jl/blob/013feadf95736639cdedb1bff2c4f1723f7dfe64/src/solvers/gradient_descent.jl#L233C1-L233C45

which “turns” a factory into the concrete thing (here the stepwise) and is the identity if one already provides a stepwise.

kwargs...)
opts = gradient_descent(M,
loss,
Expand Down Expand Up @@ -174,7 +174,7 @@ function call_manopt_optimizer(M::Manopt.AbstractManifold,
stepsize = WolfePowellLinesearch(M;
retraction_method = retraction_method,
vector_transport_method = vector_transport_method,
linesearch_stopsize = 1e-12),
stop_when_stepsize_less = 1e-12),
kwargs...
)
opts = quasi_Newton(M,
Expand Down Expand Up @@ -308,7 +308,7 @@ function call_manopt_optimizer(M::ManifoldsBase.AbstractManifold,
stopping_criterion::Union{Manopt.StoppingCriterion, Manopt.StoppingCriterionSet},
evaluation::AbstractEvaluationType = InplaceEvaluation(),
retraction_method::AbstractRetractionMethod = default_retraction_method(M),
stepsize::Stepsize = DecreasingStepsize(; length = 2.0, shift = 2),
stepsize::Stepsize = DecreasingLength(; length = 2.0, shift = 2),
kwargs...)
opt = Frank_Wolfe_method(M,
loss,
Expand Down
2 changes: 1 addition & 1 deletion lib/OptimizationManopt/test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ end
x0 = zeros(2)
p = [1.0, 100.0]

stepsize = Manopt.ArmijoLinesearch(R2)
stepsize = Manopt.ArmijoLinesearch()
Copy link
Contributor

@kellertuer kellertuer Apr 29, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@oscardssmith oscardssmith Apr 29, 2025

Choose a reason for hiding this comment

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

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?

opt = OptimizationManopt.GradientDescentOptimizer()

optprob_forwarddiff = OptimizationFunction(rosenbrock, Optimization.AutoEnzyme())
Expand Down
Loading