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

indirect solve prototype #121

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

indirect solve prototype #121

wants to merge 5 commits into from

Conversation

goulart-paul
Copy link
Member

@goulart-paul goulart-paul commented Mar 10, 2023

A small experiment to see how we could go about enabling indirect KKT solves. This PR so far does the following:

  • adds a registration mechanism for AbstractKKTSolver implementations. This uses a Dict object implemented similarly to the registration method for the direct LDL solvers. This requires a minor change to the Settings structure, which now defines a default kkt_solver_method = :directldl to get the standard solver behaviour.

  • adds a new prototype implementation for a IndirectKKTSolver solver type, which can be selected via the option kkt_solver_method = :indirect. This is very basic and assumes internally that it will only be applied to a problem with a single NN cone constraint. It does not actually implement an indirect solve, but rather reduces the KKT system to a dense form to which an an actual indirect linear solver could be applied.

  • makes some internal modifications to the rest of the code to support the indirect solve, mostly by passing cones around where they are required.

Note that if an indirect method is used then there is no reason that SparseMatrixCSC format is required, since in that case the solver only needs to perform matrix multiplication. (Almost) everything is implemented internally in terms of AbstractMatrix{T}, so the solver should work on any input type that implements enough of AbstractMatrix{T} interface. See example below for further explanation.

This is WIP and should not be used in anger. It is currently unclear to me how this could be applied to general cone problems since it requires the inverse of the scaling block Hs, or at least multiplication by the inverse of that block. See internal comments in the kktsolver_indirect.jl implementation for further explanation.

Example code:

using Clarabel, SparseArrays

# A small box constrained QP problem.  
n = 4
P = sparse(I(n)).*1.
q = collect((1:n).*1.)
A = sparse([I(n);-I(n)].*1.)
b = ones(2*n)
cones = [Clarabel.NonnegativeConeT(2*n)]


# Solve the problem using standard direct method

settings = Clarabel.Settings(kkt_solver_method = :directldl)
solver   = Clarabel.Solver(settings)
Clarabel.setup!(solver, P, q, A, b,cones)
result = Clarabel.solve!(solver)

# Solve the problem using an indirect method

settings = Clarabel.Settings(kkt_solver_method = :indirect)
solver   = Clarabel.Solver(settings)
Clarabel.setup!(solver, P, q, A, b,cones)
result = Clarabel.solve!(solver)


# Since the :indirect method doesn't require direct 
# assembly or factorization of the KKT matrix in sparse 
# format, everything should also work for matrix data 
# of other type, since (nearly) all of the solver is 
# implemented in terms of AbstractMatrix{T}.  The only 
# exceptions are :

# - info printing provides a nonzero count, so make 
# a blanket implementation for that function here
SparseArrays.nnz(A::AbstractMatrix) = prod(size(A))


# Now the solver should work with dense matrices 

settings = Clarabel.Settings(
    kkt_solver_method  = :indirect,
)

denseP = Matrix(P)
denseA = Matrix(A)
solver   = Clarabel.Solver(settings)
Clarabel.setup!(solver, denseP, q, denseA, b, cones)
result = Clarabel.solve!(solver)

@codecov-commenter
Copy link

codecov-commenter commented Mar 16, 2023

Codecov Report

Patch coverage: 66.80% and project coverage change: -1.29 ⚠️

Comparison is base (50b0c6e) 82.22% compared to head (f9fbcd3) 80.94%.

❗ Current head f9fbcd3 differs from pull request most recent head 2b58462. Consider uploading reports for the commit 2b58462 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #121      +/-   ##
==========================================
- Coverage   82.22%   80.94%   -1.29%     
==========================================
  Files          37       39       +2     
  Lines        2701     2829     +128     
==========================================
+ Hits         2221     2290      +69     
- Misses        480      539      +59     
Impacted Files Coverage Δ
src/kktsolvers/kktsolver_indirect.jl 0.00% <0.00%> (ø)
src/settings.jl 8.00% <0.00%> (ø)
src/variables.jl 77.64% <ø> (+11.64%) ⬆️
src/utils/mathutils.jl 69.64% <25.00%> (-7.56%) ⬇️
src/types.jl 85.71% <70.00%> (-0.65%) ⬇️
src/kktsolvers/direct-ldl/directldl_defaults.jl 21.42% <75.00%> (+21.42%) ⬆️
src/kktsolvers/kktsolver_defaults.jl 21.42% <75.00%> (+21.42%) ⬆️
src/info_print.jl 95.91% <92.30%> (-0.93%) ⬇️
src/presolver.jl 94.73% <94.73%> (ø)
src/Clarabel.jl 100.00% <100.00%> (ø)
... and 11 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

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