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

Adaptive Gradient Descent Method for e.g., BCG #549

Merged
merged 64 commits into from
Mar 4, 2025
Merged

Adaptive Gradient Descent Method for e.g., BCG #549

merged 64 commits into from
Mar 4, 2025

Conversation

pokutta
Copy link
Member

@pokutta pokutta commented Feb 17, 2025

Implementation of the two adaptive gradient descent methods from https://arxiv.org/abs/2308.02261 for usage with e.g., BCG and other methods that use gradient descent as subroutine.

Current version is the unconstrained one that should be sufficient for our applications

@pokutta pokutta self-assigned this Feb 17, 2025
@pokutta
Copy link
Member Author

pokutta commented Feb 17, 2025

includes algorithm, example, and test file

@pokutta
Copy link
Member Author

pokutta commented Feb 17, 2025

please also check that the example is contained in the example test -> not sure what triggers it and where it needs to be registered

@pokutta
Copy link
Member Author

pokutta commented Feb 17, 2025

some tests are failing but they seem unrelated to the adaptive gradients but happening in BPCG with many "useless steps" - similar to what we have experienced in the comp context @matbesancon @dhendryc . Did we have recent changes to BPCG that might have created this?

@dhendryc
Copy link
Collaborator

some tests are failing but they seem unrelated to the adaptive gradients but happening in BPCG with many "useless steps" - similar to what we have experienced in the comp context @matbesancon @dhendryc . Did we have recent changes to BPCG that might have created this?

This should hopefully fix it.
#550

@pokutta
Copy link
Member Author

pokutta commented Feb 19, 2025

Finally solution for CTRL-C #159 via the best intern!

@matbesancon
Copy link
Member

the try catch block is a solution but we should be careful with it, top of my head it can have a significant performance impact

@matbesancon
Copy link
Member

I replaced the ad-hoc prox with the function defined in ProximalCore, it's lightweight enough and it means we can have all the proxes in the world through the ProximalOperators added in the extension.

The only ones I kept are identity and the simplex prox for BCG

@pokutta
Copy link
Member Author

pokutta commented Feb 19, 2025

the try catch block is a solution but we should be careful with it, top of my head it can have a significant performance impact

yes indeed - we were thinking of building a macro wrapper that only sets a flag "interrupted" that we catch in the while statement and also have a flag that fully deactivates it. we want to push this also to downstream codes as boscia similar to scip and gurobi. @dhendryc @matbesancon

@pokutta
Copy link
Member Author

pokutta commented Feb 19, 2025

i am getting lots of precompile warnings and errors now:

Info Given FrankWolfe was explicitly requested, output will be shown live
WARNING: Method definition prox!(Any, Any, Any, Any) in module FrankWolfe at /Users/x_bas/Documents/Mathematik/projects/2022_branchWolfe/FrankWolfe.jl/src/gradient_descent.jl:412 overwritten at /Users/x_bas/Documents/Mathematik/projects/2022_branchWolfe/FrankWolfe.jl/src/gradient_descent.jl:415.
ERROR: Method overwriting is not permitted during Module precompilation. Use __precompile__(false) to opt-out of precompilation.
? FrankWolfe
[ Info: Precompiling FrankWolfe [f55ce6ea-fdc5-4628-88c5-0087fe54bd30] (cache misses: include_dependency fsize change (2), incompatible header (8), mismatched flags (2))
WARNING: Method definition prox!(Any, Any, Any, Any) in module FrankWolfe at /Users/x_bas/Documents/Mathematik/projects/2022_branchWolfe/FrankWolfe.jl/src/gradient_descent.jl:412 overwritten at /Users/x_bas/Documents/Mathematik/projects/2022_branchWolfe/FrankWolfe.jl/src/gradient_descent.jl:415.
ERROR: Method overwriting is not permitted during Module precompilation. Use __precompile__(false) to opt-out of precompilation.
┌ Info: Skipping precompilation due to precompilable error. Importing FrankWolfe [f55ce6ea-fdc5-4628-88c5-0087fe54bd30].
└ exception = Error when precompiling module, potentially caused by a precompile(false) declaration in the module.

@matbesancon

@matbesancon
Copy link
Member

Last thing needed is removing the tests on the prox operators since these are now in ProximalOperators.jl and replace the call to the prox functions

@matbesancon matbesancon requested review from sebastiendesignolle and removed request for matbesancon February 26, 2025 16:17
Copy link
Collaborator

@sebastiendesignolle sebastiendesignolle left a comment

Choose a reason for hiding this comment

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

Looks good to me! I'm made some minor modifs, nothing important.

@matbesancon
Copy link
Member

Thanks! Was there a reason for d6c9335
? Otherwise I'd just revert it, the spaces don't seem necessary?

@sebastiendesignolle
Copy link
Collaborator

The goal is to have the tests messages aligned, but I acknowledge that's mostly a symptom of my neurosis...

@matbesancon matbesancon merged commit 7955da7 into master Mar 4, 2025
12 checks passed
@matbesancon matbesancon deleted the AdaGD branch March 4, 2025 10:26
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.

4 participants