Implement externally owned vector and matrices

[traversaro]

To support reading and writing from and to Vector and Matrices already allocated in a form different from iDynTree::VectorDynSize, iDynTree::MatrixDynSize, iDynTree::VectorFixSize, iDynTree::MatrixFixSize (such as Eigen::MatrixXd, std::vector<double> , std::array<double>, raw c-buffers, numpy matrices, etc etc, it may be useful to define classes to represent Vector and Matrices externally owned.

While the matrix implementation is probably more complicated, the Vector implementation could be probably as simple as:

namespace iDynTree
{
class VectorExt
{
private:
    double * m_data;
    size_t m_size;
public:
    VectorExt(double * data, size_t size); 
}
}

The most used methods of the classes such as KinDynComputations and InverseKinematics can be modified to accept this VectorExt class, to simplify interoperability and reducing the need of unnecessary copy, while avoiding dealing directly with raw pointers and preserving the possibility of checking sizes.

A thing that needs to be check is how to properly differentiate external buffers that can be modified to external buffers that can be only read.

[traversaro]

Related discussions:

• [WIP] Add setExternal in ManagedBytes yarp#1582 (review)
• Vector and Matrix with compile-time known size yarp#76
• [wholeBodyInterface] For every input/output parameter explictly state in documentation the expected size robotology-legacy/codyco-modules#26 (comment)
• https://abseil.io/tips/1 (std::string_view is basically the same concept, but applied to strings)

[traversaro]

Related discussion at C++ standard committee :

• array_view proposal (generalization of what proposed here to multidimensional arrays, including aspect of stride, now abandoned due to its complexity): http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3851.pdf
• span proposal: similar to array_view, but only for 1D sequences: http://open-std.org/JTC1/SC22/WG21/docs/papers/2016/p0122r1.pdf (related abseil tip: https://abseil.io/tips/93 )
• mdspan proposal: latest proposal (i.e. evolution of array_view : https://github.com/kokkos/array_ref

[S-Dafarra]

Regarding the vector, it seems to me that VectorDynSize has already the structure of VectorExt. Do you think it may be a possibility to implement "maps" as methods of VectorDynSize?

In this case

A thing that needs to be check is how to properly differentiate external buffers that can be modified to external buffers that can be only read.

would still be an open issue. A possibility could be to use a private flag to discern whether the vector can be resized or not (using asserts similarly to what Eigen does for the maps).

On the other hand, we would not need to change any interface.

[traversaro]

Regarding the vector, it seems to me that VectorDynSize has already the structure of VectorExt. Do you think it may be a possibility to implement "maps" as methods of VectorDynSize?

VectorDynSize supports the resize method, and so it cannot be adapted to be a "VectorExt" without changing drastically its meaning and probably breaking a lot of code that use it.

A possibility could be to use a private flag to discern whether the vector can be resized or not (using asserts similarly to what Eigen does for the maps).

I check the proposal, and the std::span proposal is quite reasonable and address also these points , and will be probably accepted in C++20. A possible strategy is to just copy a liberally licensed existing implementation of span or implement and class with the same interface.

On the other hand, mdspan (the equivalent of span for n-order tensors, for n greater than one) is quite complicated and it is not clear if it will be eventually integrated in the standard, so I am afraid that rolling our solution for it may be the best option.

[S-Dafarra]

Regarding span these are two available implementations:

• https://github.com/abseil/abseil-cpp/blob/master/absl/types/span.h in abseil as you suggested
• https://github.com/martinmoene/gsl-lite/blob/master/include/gsl/gsl-lite.hpp#L1212 in gsl-lite

[traversaro]

The abseil one is Apache 2-licensed, so it is not directly incorporable in the iDynTree codebase (see https://www.dwheeler.com/essays/floss-license-slide.html). Furthermore, I have the impression that extracting the span implementation from the complete class would be quite complicated.

The gsl-lite one is MIT-licensed, so we can incorporate it directly in the iDynTree. I think we can make iDynTree C++14 only at this point (we discussed about this at a YARP level, so it should not be a problem). The gsl::span seems quite indipendent, cleaning up the gsl specific stuff (gsl_api-> , gsl_constexpr14 --> constexpr, etc etc) I think it could be include in iDynTree as iDynTree::Span (I think we can copy the capitalization from abseil to minimize the risk of collisions in code that use both using namespace iDynTree and using namespace std.

[S-Dafarra]

I think it could be include in iDynTree as iDynTree::Span (I think we can copy the capitalization from abseil to minimize the risk of collisions in code that use both using namespace iDynTree and using namespace std

👍

The gsl::span seems quite indipendent, cleaning up the gsl specific stuff (gsl_api-> , gsl_constexpr14 --> constexpr, etc etc)

Were you thinking about a rough copy/paste or wrapping it as iDynTree::Span?

[traversaro]

Copy/paste from gsl + cleanup of gsl stuff + renaming as iDynTree::Span.
After that, we can start using it in the interfaces as iDynTree::Span.

[traversaro]

Microsoft's GSL also has a MIT-license implementation of span : https://github.com/Microsoft/GSL/blob/master/include/gsl/span .

[traversaro]

cc @diegoferigo I suspect this could be useful for wb-toolbox .

[S-Dafarra]

Microsoft's GSL also has a MIT-license implementation of span :

I was looking at this table and I did not realize what M stood for 😅

It seems to be much simpler!

[traversaro]

Yes, because it is already C++14.

I think we can drop the methods returning using the std::byte, substitute Expects with std::assert and do a general cleanup and probably that one is already read to be a good iDynTree::Span .

[S-Dafarra]

Regarding narrow_cast see microsoft/GSL#167

[traversaro]

I think we can also drop a few constexpr if that is a problem with Visual Studio 2015.

[S-Dafarra]

narrow_cast can be changed easily into static_cast (see https://github.com/Microsoft/GSL/blob/master/include/gsl/gsl_util#L92), while narrow is a bit more tricky since it checks that the value has not changed after the cast (see https://github.com/Microsoft/GSL/blob/master/include/gsl/gsl_util#L110). Does it make sense to keep this check?

[traversaro]

I think we can just switch to static_cast.

[traversaro]

But the actual narrow call is quite small, so we can copy also that if you feel strongly about it.

[S-Dafarra]

Considering that in iDynTree we have only vector and matrices as containers, I wonder whether it is better to avoid considering a generic multi-span class while rather using something that eases the passage from iDynTree to Eigen and eventuallymatio 🤔 . In this sense, we could use something similar to span with some additional field to discern, for example, whether it is row or column major.

[traversaro]

I totally agree, especially because the ndspan proposal seems to be quite complicated and its inclusion in the standard is uncertain.

[S-Dafarra]

I would call this new class something like matrix_view or similar, following the string_view style. Regarding the implementation, I don't know whether there is some implementation we could already use. If we are going to implement it from scratch, I think I will not be able to have the same performances of span 😅

[traversaro]

I think I will not be able to have the same performances of span

I don't know, I don't think we can get too wrong passing a pointer and two integers around. : )
For the _view name I don't know, I read somewhere in all the standard proposal that _view was associated with the concept of read-only reference, while _ref was associated with the concept of a reference that could also have write support. Indeed, the original name for ndspan was arrray_ref .

[traversaro]

Map is also quite an interesting suffix, given that it is used in Eigen so a lot of developers would be familiar with the concept.

[S-Dafarra]

I was referring to all the machinery that makes span and make_span working effectively with a "general" vector.

Regarding the name I like the suffix Map. What about MatrixMap?

[traversaro]

MatrixMap

👍
Assuming that we agree that we don't think to handle the stride stuff at this point, I think the trickiest part is deciding if the row or column major information is a member that can change at runtime, or a template parameter specified at compile time. The compile-time route is attractive for compatibility with Eigen, but it may be problematic for declaring non-header only methods that can take in input both variants.

[S-Dafarra]

Assuming that we agree that we don't think to handle the stride stuff at this point

👍

The compile-time route is attractive for compatibility with Eigen, but it may be problematic for declaring non-header only methods that can take in input both variants.

Sincerely I prefer the compile-time option, also because I don't know if it makes sense for this information to change at run-time. Looking at the span code I saw line https://github.com/Microsoft/GSL/blob/master/include/gsl/span#L333. If I have undestood correctly (and I have big doubts about that) the public member value_type gives you information about the type of value stored, even if this information comes from the template. I was thinking of doing the same, i.e. accept the type RowMajor or ColumnMajor from template, while exposing a public member referring to it. Do you think this may solve the issue you pointed out?

Maybe we can add for our current definition of MatrixStorageOrdering a similar implementation of std::is_same in order to check which ordering is actually used, maybe using something like:

constexpr bool isRowMajor() const noexcept { return iDynTree::is_same<iDynTree::RowMajor, order_type>::value; }

where order_type is the public member I was mentioning before.

[prashanthr05]

An example for iDynTree::Span in action for the use-case of getting and setting states,

bool getState(iDynTree::Span<double>& x) const
{
    x(0) = 0.0;
    return true;
}

bool setState(iDynTree::Span<double>& x)
{
    // ... check if x and internal state have same size, else throw error
    // avoiding all that here
   double state = x(0);
   return true;
}

can be called using,

iDynTree::VectorDynSize state;
state.resize(1);
state(0) = 10.0;
std::cout << state.toString() << std::endl; // prints 10.0
iDynTree::Span<double> buf(state.data(), state.size());
getState(buf);
std::cout << state.toString() << std::endl; // prints 0.0 

state(0) = 10.0;
iDynTree::Span<double> setBuf(state.data(), state.size());
setState(setBuf);

[traversaro]

Exactly! I would however add asSpan methods to VectorDynSize and VectorFixSize, so you could just do:

setState(state.asSpan())

[S-Dafarra]

Actually the make_span method works already for iDynTree vectors. You could do

setState(iDynTree::make_span(state))

[traversaro]

You are right, I totally forgot about it!

[traversaro]

A self-declared "production-ready" implementation of mdspan is available at https://github.com/kokkos/mdspan .

[GiulioRomualdi]

I think we can close this issue right?

@traversaro

[traversaro]

Fixed by:

• Implement the MatrixView class #734
• Add Span class #434

Thanks @S-Dafarra @GiulioRomualdi !