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

Terminal cost necessary in TrajOptProblem? #44

Open
stephane-caron opened this issue Feb 27, 2023 · 5 comments
Open

Terminal cost necessary in TrajOptProblem? #44

stephane-caron opened this issue Feb 27, 2023 · 5 comments
Labels
question Further information is requested

Comments

@stephane-caron
Copy link
Contributor

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
Copy link
Member

ManifoldFR commented Feb 27, 2023

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
Copy link
Contributor Author

stephane-caron commented Feb 28, 2023

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
Copy link
Member

ManifoldFR commented Feb 28, 2023

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
Copy link

Bambade commented Feb 28, 2023

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 ManifoldFR added the question Further information is requested label Nov 7, 2023
@ManifoldFR
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants