search

utiasDSL / safe-control-gym

PyBullet CartPole and Quadrotor environments—with CasADi symbolic a priori dynamics—for learning-based control and RL
https://www.dynsyslab.org/safe-robot-learning/
MIT License
636 stars 132 forks source link

MPC warm-starting and acados implementation #161

Closed MingxuanChe closed 2 weeks ago

MingxuanChe commented 2 months ago

This pull request has the recent updates for the MPC controller.

It focuses on accelerating the online MPC runtime, including warm-starting and an acados implementation.

  1. MPC:

    • update optimizer setup with different solver options.
    • add initial guess computation for MPC warm-starting

  2. LinearMPC:

    • update for solver setup compatibility

  3. MPC_ACADOS:

    • add initial implementation
    • add example config
    • update README about acados-related additional requirement

  4. utils and mpc_utils

    • add timing decorator
    • add MPC open-loop solution plotting

  5. Miscellaneous

    • colored print-outs for MPC controllers and timing deco
adamhall commented 2 months ago
  1. There already seems to be a ton of duplication in the MPC examples, like the stabilization and tracking yamls being identical. Your acados ones add even more duplication. there has to be a way to make it way simpler

I wonder if for the examples the duplication is okay, simply because if someone wants to use a very specific function of the gym, they can immediately see an example for that function, as opposed to having to understand why all three examples are the same?

  1. Although i don't dislike the way you've done the MPC_ACADOS subclass, in general I dont like the idea of one of the controllers being just a different solver of another one of the controllers. Not only is it not really a separate controller, but it adds a lot of duplicated code. In my own code I made acados work by adding separate setup and solving functions in the same class: https://github.com/Federico-PizarroBejarano/safe-control-gym/blob/99142ae57663c23aec204931e38aadc06883bdf6/safe_control_gym/safety_filters/mpsc/mpsc.py#L121. If you think this would be uglier than your current approach, that is fine, just something to consider.

This is a good point Fed, but I think I am mostly okay with it being a separate controller. ACADOS is more than just a different solver, its an entirely different parser built on top of CasADi specifically for MPC giving access to more MPC-specific solvers. On the other hand, the structure you have is cleaner overall IMO.

  1. How does this PR affect GP_MPC? I think the renaming of the initial guess function directly impacts it, although i have never used it and I assume you know better. Just wanted to ensure there are no unintended consequences

There is another PR for GPMPC with ACADOS nearly ready, we wanted to get this one in first.

Federico-PizarroBejarano commented 2 months ago

@adamhall understood. I am happy to leave those three issues as they are in the PR

MingxuanChe commented 1 month ago

Thank you both for the comments! I tried to address the most of the comments and there are still several terms I might need some further discussion.

The remaining itesm for this PR:

  1. whether we remove .run methods
  2. whether we delete common_metric
  3. handeling when MPC solver fails (throw an error or not?)

Please let me know if I missed something.

Federico-PizarroBejarano commented 1 month ago

Hmmm unfortunately I guess its hard to add tests since it would require installing ACADOS on GitHub :(. Annoying, can't think of a good work around