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

Remove qpOASES from the Dynamics component and the superbuild? #239

Closed
traversaro opened this issue Aug 23, 2019 · 27 comments
Closed

Remove qpOASES from the Dynamics component and the superbuild? #239

traversaro opened this issue Aug 23, 2019 · 27 comments

Comments

@traversaro
Copy link
Member

traversaro commented Aug 23, 2019

The qpOASES library served us well for years, but it is a bit complicated to maintain for these reasons:

The OSQP QP solver recently emerged and is a well maintained and with an active upstream library, that does not have the problem of the build systems and of bugs of qpOASES. OSQP is already part of the superbuild. Ideally, we should be able to just use OSQP for all our QP needs, removing the need of the qpOASES and reducing the compilation time of the superbuild and our maintenance burden.

Currently this is blocked by the following issues:

@traversaro
Copy link
Member Author

If anyone thinks that for any reason qpOASES should be maintained inside the superbuild, feel free to comment, thanks!

@traversaro
Copy link
Member Author

cc @DanielePucci

@traversaro
Copy link
Member Author

traversaro commented Aug 25, 2019

It turns out that sometimes OSQP "generates a solution that is outside the boundary. I've noticed this behaviour when the problem is ill conditioned", see robotology-dependencies/qpOASES#4 (comment) . To be completely honest, if we are not able to reproduce the error I would not consider this as an issue blocking us from completly migrate to OSQP, at least in the default builds of the superbuild.

@arocchi
Copy link

arocchi commented Nov 21, 2019

I had the same experience with OSQP, but to be honest I did not try again after the report here osqp/OSQP.jl#47
My problem could have even been different altogether, as I remember the issue was that because of the relatively large eps_rel, the algorithm would converge with huge residuals.

@traversaro
Copy link
Member Author

Thanks for chiming in and for the pointers @arocchi !
@GiulioRomualdi @MiladShafiee If the problem you experienced is indeed related to osqp/osqp#151 , possibly disabling adaptive_rho (if I understood correctly) should avoid the problems, but I am not sure about the other effects of it (increase in the computational time?)

@traversaro
Copy link
Member Author

Other reports of problems on which OSQP fails: tesseract-robotics/trajopt#146 (comment) .

@traversaro
Copy link
Member Author

@robotology/iit-dic As it seems that we are not able to gain consensus of the usefulness of moving away from qpOASES, to reduce the maintenance burden it would be at least useful to remove the compilation of the qpOASES Matlab bindings from the ROBOLOTY_USES_MATLAB option.
The only public use of matlab bindings of qpOASES is in a deprecated package, but I noticed a few use in private repos of @Giulero @isorrentino @gabrielenava @lrapetti .

If we want to keep it around, I am afraid I need someone to take responsibility of the repo https://github.com/robotology-dependencies/qpOASES as a maintainer. cc @DanielePucci

@lrapetti
Copy link
Member

The only public use of matlab bindings of qpOASES is in a deprecated package, but I noticed a few use in private repos of @Giulero @isorrentino @gabrielenava @lrapetti .

If we want to keep it around, I am afraid I need someone to take responsibility of the repo https://github.com/robotology-dependencies/qpOASES as a maintainer.

Just as a side note, I think that robotology/wb-toolbox#180 would deprecate the usage for all the four of us (and might be somehow less expensive at the end that maintaining robotology-dependencies/qpOASES)

@traversaro
Copy link
Member Author

Just as a side note, I think that robotology/wb-toolbox#180 would deprecate the usage for all the four of us (and might be somehow less expensive at the end that maintaining robotology-dependencies/qpOASES)

Great!

@traversaro
Copy link
Member Author

Actually the usage of qpOASES matlab bindings that I found was actually due to @Yeshasvitvs, but I think they are just old controllers.

@gabrielenava
Copy link
Collaborator

gabrielenava commented Jan 31, 2020

From my side:

We are going to handle this modification (we = @gabrielenava @Giulero @bemilio) so that all the (still used) above mentioned controllers are exposed to QPOASES only through WBTollbox, and they will comply to any future decision concerning QPOASES.

@traversaro
Copy link
Member Author

Thanks @gabrielenava !

@fiorisi
Copy link
Member

fiorisi commented Jan 31, 2020

cc @RAR1989

@isorrentino
Copy link

I use qpOASES Matlab bindings for the insoles calibration, but I'll change the solver as soon as possible. From my side, there aren't problems with removing it.

@traversaro
Copy link
Member Author

Periodic remainder that qpOASES is a source of problems, see for example #418 .

@traversaro
Copy link
Member Author

traversaro commented Jun 12, 2020

Given the recurring problems related to qpOASES and the need to maintain its CMake build system, unless someone volunteers to maintain qpOASES my plan is to remove qpOASES from the superbuild by the end of 2020.

cc @robotology/iit-dynamic-interaction-control

@diegoferigo
Copy link
Member

From the WB-Toolbox side I think there're won't be many problems. If qpOASES is not found, the code of the block is not compiled (but the block shows up in the library, though it will not work).

@traversaro
Copy link
Member Author

From the WB-Toolbox side I think there're won't be many problems. If qpOASES is not found, the code of the block is not compiled (but the block shows up in the library, though it will not work).

I hope someone will migrate the WB-Toolbox block to osqp/OsqpEigen by then. : )

@diegoferigo
Copy link
Member

I hope so too :)

@nunoguedelha
Copy link
Collaborator

Since I will be working on robotology/blockfactory#59, I might have some time to fit this migration in my tasks...

@traversaro
Copy link
Member Author

As qpOASES is still used in WB-Toolbox, I think a good compromise to reduce the maintenance burden is to disable compilation of MATLAB bindings, that are the part that use a custom fork and the one that gave most of the problems. Furthemore, MATLAB bindings for a state of the art QP solver will be added in #585, so anyone interested can use those. Unless someone objects on this, I would proceed with this change in 7 days.

@traversaro
Copy link
Member Author

To clarify, this will not change the fact that qpOASES has well known shortcomings (i.e. feasible QP problems that are classified as unfeasible) described in coin-or/qpOASES#85 that could kick in at any moment (even when controlling robot in real world experiments) and for that working on robotology/wb-toolbox#180 could make sense.

@diegoferigo
Copy link
Member

diegoferigo commented Feb 8, 2021

After 1.5 years from the publishing of this issue few things have changed in the lab, I noticed that the usage of CasADI significantly increased. Thanks to @GiulioRomualdi, after #526 the superbuild can optionally compile CasADI from sources, and by default it enables the IPOPT and (updated) OSQP backends.

Having as many available backend as possible is imho a good idea, regardless of the limitations of qpOASES. What's important is being aware of them. This being said, if the maintenance burden comes from the need of MATLAB bindings and if without them we can switch back to using upstream, I would like to discuss about keeping qpOASES inside the superbuild.

I recently found useful this version of CasADI to get out-of-the-box a complete Python package, that is similar to the one published in PyPI but built from sources against the system's libraries. Unfortunately also CasADI (as many other C++ hybrid packages) suffers from incompatibilities when loading multiple C++ modules casadi/casadi#1012, and a source compilation often helps.

@traversaro
Copy link
Member Author

Yes, I have no problem in having qpOASES in the superbuild if we can use the upstream repo (and dropping MATLAB bindings is the first step in that direction) and as long users are aware that may not a good idea to use it by default.

@traversaro
Copy link
Member Author

On a related note, if someone wants to enable qpOASES in the CasADi build of the superbuild, that's a good idea as well.

@diegoferigo
Copy link
Member

On a related note, if someone wants to enable qpOASES in the CasADi build of the superbuild, that's a good idea as well.

Yep I agree, we should pass -DWITH_LAPACK=ON WITH_MUMPS=ON WITH_QPOASES=ON.

Then, since we're there, by passing WITH_PYTHON=ON WITH_PYTHON3=ON we can enable python support. There are two caveats:

  1. The module could not be imported. I had to apply the following patch: sed -i "s/class _copyableObject(_object):/class _copyableObject():/g" /usr/local/src/robotology-superbuild/build/install/python/casadi/casadi.py. I guess it is a Python2 related problem.
  2. The Python bindings are installed into <build>/install/python.

Maybe this is not the right place where to write this info, but for the moment let's log it here.

@traversaro
Copy link
Member Author

In coin-or/qpOASES#108 and coin-or/qpOASES#109 I contributed upstream the modification of qpOASES that we need, and after that we can switch to use qpOASES upstream (just waiting on a proper release, see coin-or/qpOASES#113). In any case, given that at this point we do not have anymore mantainance overhead coming from qpOASES, I think we can close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants