robotology / idyntree

Multibody Dynamics Library designed for Free Floating Robots
BSD 3-Clause "New" or "Revised" License
177 stars 67 forks source link

Add the possibility to use iDynTree span as input/output for KinDyn computation object #716

Closed GiulioRomualdi closed 4 years ago

GiulioRomualdi commented 4 years ago

This feature it will be very useful if we are going to use other libraries (i.e. Eigen) as input objects for KinDyn

As regards vectors this can be easily handled by using the iDynTree::Span. For matrices the problem is more complex this could be a possible solution

traversaro commented 4 years ago

For matrices the problem is more complex this could be a possible solution

I think that mdspan is quite complex, so for the time being we could just roll out our own simple iDynTree::MatrixView with a limited subset of functionality we are interested (ColMajor/RowMajor, no strange stride) and start using that. When and if the mdspan reaches the standard, then we can start using it.

GiulioRomualdi commented 4 years ago

As far as I understood the iDynTree matrices are row-major only (I'm talking about Dense matrices)

traversaro commented 4 years ago

As far as I understood the iDynTree matrices are row-major only (I'm talking about Dense matrices)

Yes, but if you want to permit to user to do:

Eigen::Matrix3d eigenMat;
function_that_takes_in_input_a_matrix(asSpan(eigenMat))

we need to support also col major matrix in iDynTree::MatrixView .

GiulioRomualdi commented 4 years ago

The code we developed in bipedal-locomotion-framework uses Eigen for storing matrices and vectors. Right now I need a massive use of kinDynComputation object and I would like to start working on this. As far as I understood there are several problems that may arise with implementation:

  1. We can use iDynTree::Span only for vectors, in case of matrices we may need to implement iDynTree::MatrixView https://github.com/robotology/idyntree/issues/716#issuecomment-656615912, which I don't have any idea on how to implement it. If @traversaro may point me something, it would be really appreciated.
  2. Using iDynTree::Span, iDynTree::MatrixView we cannot resize the quantities. In KinDynComputation the vectors/matrices are resized several times.
  3. The iDynTree::Tranform and iDynTree::Twist does not store the information in contiguous arrays. It may be complex to "spannify" them.
GiulioRomualdi commented 4 years ago

This is the first implementation of the MatrixView, tomorrow I will open the PR (in draft) and I'll try to interface it with KinDynComputation

traversaro commented 4 years ago

If @traversaro may point me something, it would be really appreciated.

Sorry, just read this.

Using iDynTree::Span, iDynTree::MatrixView we cannot resize the quantities. In KinDynComputation the vectors/matrices are resized several times.

That is not a problem. We can keep the existing resizing methods that take in input concrete vectors, and just add new equivalent methods that take in input Span/MatrixView and return an error if the span/view with the wrong size is reported.

The iDynTree::Tranform and iDynTree::Twist does not store the information in contiguous arrays. It may be complex to "spannify" them.

I think we can just choose an equivalent vector/matrix representation of them (like 4x4 homogeneous matrix or other for Transform, or 6d vector for Twist), and then copy the data in the right data structure to pass to the rest of the methods. I don't think it is worth trying to avoid at all cost copies, especially if it is just for input/output data structures.

GiulioRomualdi commented 4 years ago

This has been achieved in https://github.com/robotology/idyntree/pull/736. I think we can close the issue

traversaro commented 4 years ago

Yes, thanks!