Skip to content
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.

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

Augmented Lagrangian method #158

Merged
merged 12 commits into from
Jan 16, 2025
Merged

Augmented Lagrangian method #158

merged 12 commits into from
Jan 16, 2025

Conversation

aldma
Copy link
Contributor

@aldma aldma commented Oct 14, 2024

Dear JSOptimizers, this PR is to add an implementation of AL method for dealing with regularized problems with nonlinear (smooth) constraints.

@aldma
Copy link
Contributor Author

aldma commented Oct 14, 2024

@dpo

@aldma
Copy link
Contributor Author

aldma commented Oct 14, 2024

This implementation could take over the one in https://github.com/aldma/Bazinga.jl , which now has a slightly more flexible template but only a basic API.

Copy link
Member

@dpo dpo left a comment

Choose a reason for hiding this comment

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

Hi @aldma ! Many, many thanks for this! I just have a couple of very general comments for now, and a question: how does this solver compare to Bazinga?

src/AL_alg.jl Outdated
nlp::AbstractNLPModel,
h::H,
options::ROSolverOptions;
subsolver = has_bounds(nlp) ? TRDH : PG,
Copy link
Member

Choose a reason for hiding this comment

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

I think better choices would be TR and R2 here. R2 is typically faster than PG. TRDH will only work if h is separable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, TR and R2 are now the default subsolvers. However, it seems there is no common interface for the solvers. In particular, I think TR with a default norm would be handy.

src/AL_model.jl Outdated
@@ -0,0 +1,179 @@
# based on Percival.jl
# https://github.com/JuliaSmoothOptimizers/Percival.jl/blob/main/src/AugLagModel.jl
Copy link
Member

Choose a reason for hiding this comment

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

What are the differences with Percival's AugLagModel, if any?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an additional method project_y! used to safeguard the dual estimate, since I tend to prefer the Birgin & Martinez algorithm over the Conn, Gould & Toint implemented in Percival. Both approaches could be implemented though.
A reason for having another, dedicated AugLagModel here (other than reducing dependencies) is that we could include also the composition of the nonsmooth term with a smooth one.

Copy link
Member

Choose a reason for hiding this comment

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

we could include also the composition of the nonsmooth term with a smooth one.

What do you mean exactly? We would bundle the smooth term with the other smooth terms in order to keep the prox operators as simple as possible, wouldn't we?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Say the cost is of the form f(x)+g(F(x)) for some smooth f, F and nonsmooth g. An option is to reformulate the problem with some additional variables to avoid the composition of g with F. This approach works but there is also another option, which eliminates the auxiliary variables. Applying the AL machinery gives an AL function with a term being the Moreau envelope of g composed with a shifted F, and one always evaluates only the simple prox operator of g.
My point is, the AL function depends on the problem formulation and there are several approaches when some g appears in the problem.

@aldma
Copy link
Contributor Author

aldma commented Oct 15, 2024

Bazinga can take problems with constraints in the form c(x) \in D, where set D is easy to project onto. The AL solver here can handle these problems at the price of some additional auxiliary variables.
In terms of practical performance, I do not have any comparisons yet.

@dpo
Copy link
Member

dpo commented Nov 12, 2024

@MohamedLaghdafHABIBOULLAH @MaxenceGollier Could you please make a pass on this code too? Thanks.

src/AL_alg.jl Outdated Show resolved Hide resolved
src/AL_model.jl Outdated
@@ -0,0 +1,179 @@
# based on Percival.jl
# https://github.com/JuliaSmoothOptimizers/Percival.jl/blob/main/src/AugLagModel.jl
Copy link
Member

Choose a reason for hiding this comment

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

we could include also the composition of the nonsmooth term with a smooth one.

What do you mean exactly? We would bundle the smooth term with the other smooth terms in order to keep the prox operators as simple as possible, wouldn't we?

Copy link
Contributor

@MaxenceGollier MaxenceGollier left a comment

Choose a reason for hiding this comment

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

Thank you @aldma for this PR !

Just a general comment, we are currently trying to revamp this repository to be more compliant with the JSO style of solvers (see #139).
A few tips for your implementation

  • Avoid using options::ROSolversOptions and use documented keyword arguments instead. this makes readibility easier for everyone as some solvers use the same variable names for different things.
  • Add a solver structure (e.g mutable struct nameSolver where name is the name of your solver) that contains makes all the neccessary allocations for your solver when you construct it. The constructor should take an AbstractNLPModel as argument.
  • Write the implementation of the solver in the function SolverCore.solve!(nameSolver,...) that allocates as less as possible (ideally it should not be allocating at all).
  • Add a callback that is called at each iteration

If you need help with this, don't minding sending me messages. It would be very helpful for us. From my understanding, your implementation is similar to Percival.jl maybe you can look at how it is done there and try to work out something similar.

src/AL_model.jl Outdated
Comment on lines 1 to 2
# based on Percival.jl
# https://github.com/JuliaSmoothOptimizers/Percival.jl/blob/main/src/AugLagModel.jl
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this file should be removed. If you want to add features to the AugLagModel from Percival.jl it would be easier to just add Percival as a dependency.
If later you think your model will describe something different, then you should create a subtype of the AugLagModel.
In all cases, these implementations should either be in Percival.jl or RegularizedProblems.jl.
If the number of dependencies is an issue, maybe we could work something out with extensions though it is not clear for me how we could do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file has been removed with 3d504cf. Now there is only a (default) method for the dual safeguard. The dependency on Percival could be weakened in the future.

@aldma
Copy link
Contributor Author

aldma commented Nov 18, 2024

Thanks @MaxenceGollier for your feedback! I have revamped the AL solver following your instructions:

  • removed options::ROSolversOptions and use now the unified kwargs.
  • build ALSolver structure and implemented solve! method.
  • added a callback.

Notice I added one method to R2 too.

@MaxenceGollier
Copy link
Contributor

This looks great ! Thank you @aldma,
Could you add unit tests to see whether solve! allocates ?
Something similar to #147 perhaps ?

@dpo
Copy link
Member

dpo commented Jan 9, 2025

@MaxenceGollier I’m assigning you to this PR. @aldma will not be able to take care of it in the next few months. Would you please check allocations and add unit tests? Thank you!

@MaxenceGollier
Copy link
Contributor

No problem, will look into it.

@dpo dpo merged commit 7302327 into JuliaSmoothOptimizers:master Jan 16, 2025
1 check failed
@dpo
Copy link
Member

dpo commented Jan 16, 2025

Many thanks @aldma !

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.

3 participants