Simple-Robotics / aligator

A versatile and efficient framework for constrained trajectory optimization
https://simple-robotics.github.io/aligator/
BSD 2-Clause "Simplified" License
100 stars 15 forks source link

Terminal cost necessary in TrajOptProblem? #44

Open stephane-caron opened 1 year ago

stephane-caron commented 1 year ago

Why is the terminal cost necessary in a TrajOptProblem, while terminal constraints are optional?

In e.g. this MPC for bipedal walking there are terminal constraints but no terminal cost.

ManifoldFR commented 1 year ago

Good question ! We just ended up aping Crocoddyl's design pattern, where an ActionModelAbstract is used as a proxy for the terminal cost. All the constructors require a terminal action model there -- since here we use terminal costs directly through a shared_ptr<CostModelAbstract>, that's what we asked for.

In practice, even with this design pattern, one can use a sum of costs (CostModelSum in croco / CostStack in proxddp) with precisely zero terms to have a no-terminal-cost problem.
I'd be open to changing the API to do this automatically though!

Another option would be to have the pointer to the terminal cost be nullptr and have an if clause everywhere in the backend but I'm not sure if this is "safe" of convenient for maintainers

stephane-caron commented 1 year ago

I'd be open to changing the API to do this automatically though!

From a user's perspective this would make sense to me.

Whether it's implemented internally as a nullptr, or an optional in the Python bindings, etc., is up to you. I think ProxQP's interface already has optionals like these?

ManifoldFR commented 1 year ago

Yes, although having optionals seems to have introduced some overhead due to the way std::optional, tl::optional or whichever "optional" implementation they use is exposed via Pybind11. Maybe @Bambade can give us more up-to-date details on this.

On our side, boost::optional is unfortunately not supported by Boost.Python which makes things messy. I think Crocoddyl uses boost::python::optional to handle exposing overloaded constructors/constructors with default arguments which could be a solution.

Bambade commented 1 year ago

We indeed use both std::optional or tl::optional (via optional which checks whether or not std::optional is available, from C++ 17). Actually, it is Pybind11 overloading which causes non negligible overhead and not specifically optional implementation. Optional feature does not introduce much overhead.

ManifoldFR commented 3 days ago

Hello from the future

Eigenpy now supports std::optional and aligator is now C++17. So we can envision doing something with optional terminal costs.

PR #159 will remove storing model classes as shared_ptr so the right pattern would perhaps be optional<polymorphic<CostModelAbstract>>, however the polymorphic<T> does itself have an empty state which we can test for.