Bipedal-locomotion-framework: Bipedal-locomotion-framework and iDynTree. A love that has come to an end

Created on 6 Jul 2020  路  9Comments  路  Source: dic-iit/bipedal-locomotion-framework

Disclaimer The title of the issue is deliberately provocative.

In https://github.com/robotology/idyntree/issues/707 I show the possibility to use Eigen in iDynTree public API. The main pro of using Eigen is enabling the linear algebra.
Discussing with @traversaro we realized that even if _describing_ an iDynTree::VectorFixSize<> with Eigen is relatively simple https://github.com/robotology/idyntree/pull/709, implementing the same approach for an iDynTree::VectorDynSize is way less trivial. Indeed in iDynTree, the concept of capacity is widely used. This concept does not have a corresponding in Eigen.

In order to solve #56 and other similar problems, we (@traversaro and I) drew 2 + 1 possible solutions:

  1. Nothing changes We keep the status quo.
  2. Introduce operator+() (and all the others) in iDynTree between matrices and vector

    • Pro it should be a relatively simple work
    • Cons every time an operation is computed new memory is allocated. :cry: I would avoid this.
  3. Avoid to use iDynTree in bipedal locomotion framework:

    • Pro: If all the methods and public attributes of the classes are Eigen It will be easy to use the library in other projects. Hopefully, the library will be used also in other labs since Eigen can be considered as numpy for Python. In the past, we already used Eigen instead of iDynTree in osqp-eigen.
    • Cons: It might be a long task.

In order to tackle the second point we should write a simple interface to iDynTree::KinDynComputations that takes as input Eigen Objects. The interface becomes useless as soon as the KinDynComputations class accepts iDynTree::Span

We should make a decision as fast as possible since the code in the repository is increasing day after day. (In theory, we should decide before merging #61 #62)

@dic-iit/dynamic-interaction-control

help wanted question

Most helpful comment

All 9 comments

To be honest, it is not clear to me the outcome. Do you want to completely remove iDynTree as a dependency, or you just want to avoid using the iDynTree::VectorDynSize as much as possible? To be honest, removing iDynTree as a dependency is even trickier since right now we need Span (which can be fully exported by the way) while in the future we will need some library for dynamic computations :thinking:

The main idea is to keep iDynTree in private dependency (except for span) and use VectorXd, Eigen::Ref etc etc... in the public API.

Correct me if I'm mistaken @traversaro

Yes, the idea is that Eigen::VectorXd or Eigen::Ref may be more convenient interface type then iDynTree::VectorDynSize now that alignment issues have been resolved. I don't see any benefit in trying to replace objects that do not have an equivalence in Eigen (such as iDynTree::Span, for which instead we need to wait for C++20 and std::span).

I think this should have been the original design of the public APIs :)

I am a big fan of STL-only APIs (e.g. https://github.com/robotology/gym-ignition/pull/158 and the wearable interfaces for interfaces I recently desiged), they simplify system integration to a level that is priceless. The PIMPL idiom should also be a must. Of course, everything keeps working fine in the case only vectors are required. When there are APIs that need to accept or return matrices, a math-oriented library is often necessary. Now that Eigen solved its problems in public APIs, I will start advocating for this solution.

Discussing with @traversaro and @S-Dafarra we decided to start using the Eigen in the public API.

For this reason, I will change the code of #61 to be in accordance with this new choice. I kindly ask @S-Dafarra to do the same for #62.

For the already existing code, I start the conversion in the "free time". I will open a PR for each existing component

CC @dic-iit/dynamic-interaction-control

The choice of using Eigen in the public API of BipedalLocomotionFramework has already been assimilated through several PRs. I think we can close this issue now.

I guess that then we can close https://github.com/robotology/idyntree/issues/707 ?

Was this page helpful?
0 / 5 - 0 ratings