Skip to content

[WIP] OSQP #387

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

Draft
wants to merge 20 commits into
base: devel
Choose a base branch
from
Draft

[WIP] OSQP #387

wants to merge 20 commits into from

Conversation

Lucas-Haubert
Copy link

@Lucas-Haubert Lucas-Haubert commented Mar 19, 2025

Note: Not mergeable until the tests for all backends pass and the code is refactorized.

What is done here:

  • Code for dense backend of OSQP.
  • Bindings.

To do:

  • Pass all the tests for dense backend (eg maros meszaros).
  • Code for sparse backend.
  • Tests for sparse backend.

In particular for the dense backend:

  • Pass the tests (see next section).
  • Remove the solved the closest feasible problem option, as not usefull with OSQP (?).
  • Remove useless commentaries (once tests pass).
  • Remove code duplication (solveDenseQP in the bindings ; init and init_settings in the wrapper ;
  • Refactorize the code (once tests pass (?)).
  • Benchmarks (once the tests pass).
  • Potential engineering tasks (concerning the packaging, or others).

Details on the failing tests:

NOTE: The majority is tested with PrimalDualLDLT and no mu update, as this is the simplest case to be valid. When choosing to update may be more efficient, I did not put the option yet, as some tests are worst with it. Moreover, the algorithm should converge anyway with a fixed value of mu (?).

  • Update of mu (rho in OSQP paper) -> some results are worst than without the update, and tests do not pass (eg osqp_dense_qp_solve, test on warm starts) -> make the update of rho more robust (could not find any difference between my implementation and the one from the paper...).
  • osqp_cvxpy.cpp: Problems due to memory (more details in commentaries in the file) -> Use of valgrind or another tool ?
    Maros Mezsaros, with the simplest setting (dense backend, PrimalDualLDLT configuration) -> see following points:
  • Many tests are skipped (as for ProxQP, since the size of the problem gets above some threshold): should we test them too ? (I guess they were tested previously, but skipped to gain time in the CI or other ?).
  • Different categories of errors are listed (algorithm failing to converge with some problems): A general observation is that the polishing step make r_prim and r_dual worst when they: do not converge to 0 sufficiently with the ADMM or seem ok but the dual risudual gap (r_g in the doc) do not converge to 0.
  • Errors regarding the memory in some tests, in the polish function in lines 792 (qpwork.k_plus_delta_k_polish = qpwork.k_polish;) or 929 (hat_t = g_polish_rhs;).

@CLAassistant
Copy link

CLAassistant commented Mar 19, 2025

CLA assistant check
All committers have signed the CLA.

@Lucas-Haubert Lucas-Haubert reopened this Mar 20, 2025
@pgraverdy pgraverdy added the pr status wip To not review in weekly meeting label Mar 21, 2025
@jorisv jorisv added the pr status to review To review in weekly meeting label Mar 24, 2025
@hugtalbot hugtalbot removed the pr status wip To not review in weekly meeting label Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr status to review To review in weekly meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants