Add dense BFGS and compact LBFGS algorithms #221

frapac · 2022-09-08T13:57:16Z

Right now BFGS is implemented as a BFGSKKTSystem. I am not sure this is the right abstraction, as this leads to a lot of duplicate code. I am currently working on another abstraction, that would allow to support directly BFGS at the condensed level.

Remaining todos:

Support DenseCondensedKKTSystem with BFGS
Add proper interface to use DampedBFGS
Add support for GPU

frapac · 2022-09-09T02:45:56Z

I am currently not able to reproduce the failure in the CI locally. Investigating what's going on.

codecov-commenter · 2022-09-09T03:09:18Z

Codecov Report

Merging #221 (9d29075) into master (15c227e) will increase coverage by 1.05%.
The diff coverage is 97.79%.

@@            Coverage Diff             @@
##           master     #221      +/-   ##
==========================================
+ Coverage   74.03%   75.09%   +1.05%     
==========================================
  Files          38       39       +1     
  Lines        3871     4079     +208     
==========================================
+ Hits         2866     3063     +197     
- Misses       1005     1016      +11

Impacted Files	Coverage Δ
src/KKT/KKTsystem.jl	`97.36% <ø> (ø)`
src/MadNLP.jl	`0.00% <ø> (ø)`
src/options.jl	`94.11% <80.00%> (-2.55%)`	⬇️
src/quasi_newton.jl	`95.49% <95.49%> (ø)`
src/IPM/IPM.jl	`98.95% <100.00%> (+0.08%)`	⬆️
src/IPM/callbacks.jl	`93.80% <100.00%> (+1.85%)`	⬆️
src/IPM/factorization.jl	`100.00% <100.00%> (ø)`
src/KKT/dense.jl	`97.20% <100.00%> (-2.80%)`	⬇️
src/KKT/sparse.jl	`99.36% <100.00%> (+0.02%)`	⬆️
src/LinearSolvers/linearsolvers.jl	`69.23% <100.00%> (+49.23%)`	⬆️
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

src/IPM/callbacks.jl

src/quasi_newton.jl

mohamed82008 · 2022-10-06T14:19:20Z

src/quasi_newton.jl

@@ -262,24 +267,24 @@ function update!(qn::CompactLBFGS{T, VT, MT}, Bk, sk, yk) where {T, VT, MT}
    #            [ U₂ ]          [  U₂ ]

    # Step 1: σₖ I
-    sigma = dot(sk, yk) / dot(sk, sk)         # σₖ
-    Bk .= sigma                               # Hₖ .= σₖ I (diagonal Hessian approx.)
+    sigma = curvature(Val(qn.init_strategy), sk, yk)    # σₖ


This will likely cause a dynamic dispatch because the value of qn.init_strategy is not known at compile time. I suggest storing the Val(qn.init_strategy) directly in the qn type and giving it a type parameter.

mohamed82008 · 2022-10-06T14:33:19Z

I don't claim to understand everything in this PR but I am happy to test it once it's ready.

frapac · 2022-10-11T19:49:24Z

FYI, here is a benchmark of MadNLP LBFGS with Ipopt LBFGS algorithm (using SCALAR1 init, and mem_size=6) on a subset of the CUTEst benchmark.

https://web.cels.anl.gov/~fpacaud/result_lbfgs.txt

Overall, Ipopt LBFGS is better than MadNLP one. I am currently working on improving MadNLP LBFGS to match Ipopt's performance.

- quasi-Newton approximation of Lagrangian's Hessian - implemented as a dense KKT system

- AbstractKKTSystem is now parameterized by a HessianApproximation - add two options DENSE_BFGS and DENSE_DAMPED_BFGS - add support for DENSE_CONDENSED_BFGS - add jtprod! and jprod! function for HS15Model and DummyQPModel in MadNLPTests

reference: Byrd, Richard H., Jorge Nocedal, and Robert B. Schnabel. "Representations of quasi-Newton matrices and their use in limited memory methods." Mathematical Programming 63, no. 1 (1994): 129-156.

sshin23

Looks good, @frapac! I have made just a few comments.

To make it possible to use BFGS with JuMP, I guess we need to implement jtprod! for MOIModel. I'd suggest implementing it in a separate PR, and we can do a major release.

sshin23 · 2022-10-25T16:08:30Z

lib/MadNLPGPU/src/MadNLPGPU.jl

@@ -12,6 +12,7 @@ import .CUBLAS: handle, CUBLAS_DIAG_NON_UNIT,
 import KernelAbstractions: @kernel, @index, wait, Event
 import CUDAKernels: CUDADevice

+import MadNLP: NLPModels


we can do

import MadNLP: MadNLP, NLPModels, @kwdef, MadNLPLogger, @debug, @warn, @error, AbstractOptions, AbstractLinearSolver, AbstractNLPModel, set_options!, SymbolicException,FactorizationException,SolveException,InertiaException, introduce, factorize!, solve!, improve!, is_inertia, inertia, tril_to_full!, LapackOptions, input_type, is_supported, default_options, symul!

sshin23 · 2022-10-25T16:13:33Z

lib/MadNLPTests/src/Instances/hs15.jl

@@ -51,6 +51,26 @@ function NLPModels.jac_coord!(nlp::HS15Model, x::AbstractVector, J::AbstractVect
    return J
 end

+function NLPModels.jprod!(nlp::HS15Model, x::AbstractVector, v::AbstractVector, jv::AbstractVector)


Are we currently using this for testing? I think it would be use this instance for testing BFGS approximations

I added a test for HS15Model and LBFGS. The problem is that this instance is nonconvex, and MadNLP + exact Hessian is returning a different solution than MadNLP + LBFGS/BFGS. Maybe we should use HS71 instead?

sshin23 · 2022-10-25T16:31:42Z

src/quasi_newton.jl

+    qn.Mk .= qn.SdotS                            # Mₖ = Sₖᵀ Sₖ
+    mul!(qn.Mk, qn.Lk, qn.DkLk, one(T), sigma)   # Mₖ = σₖ Sₖᵀ Sₖ + Lₖ Dₖ⁻¹ Lₖᵀ
+    symmetrize!(qn.Mk)
+    Jk = cholesky(qn.Mk).L                       # Mₖ = Jₖᵀ Jₖ (factorization)


it is likely that we can reduce allocation by using cholesky! (might be able to optimize further by directly calling CHOLMOD, but that might be a bit too much of work). I'd recommend storing the factorization as qn.MkF and calling cholesky!(qn.MKF, qn.Mk)

This has been implemented! But I agree that medium term it would be better to use our own wrapper for CHOLMOD.

lib/MadNLPGPU/src/callbacks.jl

src/quasi_newton.jl

sshin23 · 2022-10-25T18:35:50Z

src/quasi_newton.jl

+
+    # Step 1: σₖ I
+    sigma = curvature(Val(qn.init_strategy), sk, yk)    # σₖ
+    Bk .= sigma                                  # Hₖ .= σₖ I (diagonal Hessian approx.)


no Bk[diagind(Bk)] here?

No! We use LBFGS only in sparse mode, so Bk is always a vector here

src/quasi_newton.jl

src/IPM/callbacks.jl

src/KKT/dense.jl

sshin23

Thanks for the long wait @frapac! Only have very minor comments, and it looks good!

sshin23 · 2023-03-15T18:30:46Z

src/KKT/dense.jl

    hess::MT
    jac::MT
+    qn::QN


I'd suggest naming qn as qnewton or something more indicative

sshin23 · 2023-03-15T18:41:34Z

src/KKT/sparse.jl

-    hess_I = Vector{Int32}(undef, get_nnzh(nlp.meta))
-    hess_J = Vector{Int32}(undef, get_nnzh(nlp.meta))
-    hess_structure!(nlp,hess_I,hess_J)
+    if QN <: ExactHessian


can we do multiple dispatch here instead? And I wonder if hess_I and hess_J are ver used for quasi-newton

frapac · 2023-03-16T09:14:51Z

@sshin23 Thank you for the comments! I have updated the PR accordingly, let me know if you want me to modify anything else.

And I wonder if hess_I and hess_J are ver used for quasi-newton

Indeed, we have to define the sparsity pattern of the diagonal elements for the quasi-Newton method, in order to store the elements associated to \xi I in the compact formulation.

frapac requested a review from sshin23 September 8, 2022 13:57

frapac marked this pull request as ready for review September 8, 2022 21:40

baggepinnen reviewed Sep 9, 2022

View reviewed changes

src/IPM/callbacks.jl Outdated Show resolved Hide resolved

frapac changed the title ~~Add dense BFGS algorithm~~ Add dense BFGS and compact LBFGS algorithms Oct 5, 2022

frapac mentioned this pull request Oct 5, 2022

LBFGS approximation of the inverse Hessian #39

Closed

baggepinnen reviewed Oct 6, 2022

View reviewed changes

src/quasi_newton.jl Outdated Show resolved Hide resolved

mohamed82008 reviewed Oct 6, 2022

View reviewed changes

frapac added 14 commits October 24, 2022 10:06

implement BFGSKKTSystem

b8788b4

- quasi-Newton approximation of Lagrangian's Hessian - implemented as a dense KKT system

add proper tests and clean code

96fa91e

add proper abstraction for quasi-Newton

ee84e4c

- AbstractKKTSystem is now parameterized by a HessianApproximation - add two options DENSE_BFGS and DENSE_DAMPED_BFGS - add support for DENSE_CONDENSED_BFGS - add jtprod! and jprod! function for HS15Model and DummyQPModel in MadNLPTests

fix tests

4123ee6

fix documentation

b3e7e90

comment out breaking test...

ea7f1f3

port BFGS to CUDA GPU

9734f71

use get! to access buffers in Dict

cc8fda9

add compact LBFGS formulation

ba4fa2d

reference: Byrd, Richard H., Jorge Nocedal, and Robert B. Schnabel. "Representations of quasi-Newton matrices and their use in limited memory methods." Mathematical Programming 63, no. 1 (1994): 129-156.

fix LBFGS

e6f06f8

reduce allocations in LBFGS

39e7c65

fix curvature test in LBFGS

1f1cf83

fix BFGS on GPU

468821e

fix breaking tests on MadNLP 0.5.1

6436666

frapac force-pushed the fp/bfgs branch from 2f3ea58 to 6436666 Compare October 24, 2022 15:20

frapac added 4 commits October 24, 2022 10:54

fix tests on GPU

0efe79a

additional fixes

89f7c5f

Merge remote-tracking branch 'origin/master' into fp/bfgs

9d29075

fix tests (again) on GPU

6ee735a

sshin23 approved these changes Oct 25, 2022

View reviewed changes

address PR's comments

b2e70b9

sshin23 approved these changes Mar 15, 2023

View reviewed changes

rescope PR with latest comments

c4c4afe

fix tests on GPU

40bc749

sshin23 approved these changes Mar 17, 2023

View reviewed changes

frapac merged commit f08c196 into master Mar 23, 2023

frapac deleted the fp/bfgs branch May 31, 2023 08:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dense BFGS and compact LBFGS algorithms #221

Add dense BFGS and compact LBFGS algorithms #221

frapac commented Sep 8, 2022 •

edited

Loading

frapac commented Sep 9, 2022

codecov-commenter commented Sep 9, 2022 •

edited

Loading

mohamed82008 Oct 6, 2022

mohamed82008 commented Oct 6, 2022

frapac commented Oct 11, 2022

sshin23 left a comment

sshin23 Oct 25, 2022

sshin23 Oct 25, 2022

frapac Nov 14, 2022

sshin23 Oct 25, 2022

frapac Nov 14, 2022

sshin23 Oct 25, 2022

frapac Nov 14, 2022

sshin23 left a comment

sshin23 Mar 15, 2023

sshin23 Mar 15, 2023

frapac commented Mar 16, 2023

Add dense BFGS and compact LBFGS algorithms #221

Add dense BFGS and compact LBFGS algorithms #221

Conversation

frapac commented Sep 8, 2022 • edited Loading

frapac commented Sep 9, 2022

codecov-commenter commented Sep 9, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

mohamed82008 commented Oct 6, 2022

frapac commented Oct 11, 2022

sshin23 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sshin23 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frapac commented Mar 16, 2023

frapac commented Sep 8, 2022 •

edited

Loading

codecov-commenter commented Sep 9, 2022 •

edited

Loading