Yarp: Vector typedef of VectorOf<double>

Created on 15 Mar 2018  路  19Comments  路  Source: robotology/yarp

Working on yarp::sig::Vector I noticed that it contains a VectorOf<double> and the functions are wrappers of the VectorOf's ones.

Shall we convert the Vector in a typedef of VectorOf<double> ?

Library - YARP_sig Answered

Most helpful comment

@pattacini I agree, what I meant is all what happens under the hood. I envision a world without raw pointers, call me a dreamer :joy:

All 19 comments

We should check how this interacts with the operator overloading defined in https://github.com/robotology/yarp/blob/master/src/libYARP_math/include/yarp/math/Math.h#L27 .

I'm quite confused... Why some operators of yarp::sig::Vector are inside YARP_math and not defined inside YARP_sig ?

@Nicogene the rationale being - I presume - that in YARP_math you'll get the math defined over the algebra of vectors and matrices, whereas YARP_sig provides only the serialized structures.

I see, but it is very strange since YARP_math is not compiled by default, so Vector without math has less functionalities and it doesn't seem very intuitive in my opinion.

@Nicogene just think of yarp::sig::Vector as the yarped counterpart of std::vector, somehow. In this context, I believe it does make sense, after all.

I found some reasons that makes this one a breaking change :

  1. In VectorOf we have :

https://github.com/robotology/yarp/blob/290545d37e59d5d8ae3f1c55df151c656b308152/src/libYARP_sig/include/yarp/sig/Vector.h#L160-L168

and in Vector:

https://github.com/robotology/yarp/blob/290545d37e59d5d8ae3f1c55df151c656b308152/src/libYARP_sig/include/yarp/sig/Vector.h#L387-L396

  1. toString() is only implemented in Vector. A better implementation would be a toString agnostic to the type as it has been done for the PointCloud.

  2. zero() is only implemented in Vector. It performs a memset, why not implementing it in VectorOf?

  3. subvector is only implemented in Vector.

I think that the only real breaking change will be the 1.
2, 3, 4 could be easily fixed moving some functions in VectorOf implementing them in a more general way

I think that the only real breaking change will be the 1.

I think we should add a .data() method to VectorOf, also to increase compatibility with C++11's std::vector.

@Nicogene I only forsee improvements if we menage to implement those functionalities to VectorOf.
Maybe there are reasons (?) to which they were not implemented in the first place?

Maybe we should include in the discussion @lornat75

I agree, I'm brave enough to try to clean it up :sweat_smile:

Every time I dig into this yarp meander I wonder why we're not switching to std::vector to handle the data buffer. Moving closer to the standard is always a great choice, and would let the implementation become more robust and less error prone. I would also consider std::valarray, I don't remember though if it has the data() method, which is super useful for this usage.

@diegoferigo internally we might consider using std::vector as container, but then I'd really stick to yarp::sig::Vector for what pertains to mathematical operations.

I think that there is a big misunderstanding about the Vector class... One thing is the container and one thing is the mathematical vector.

@pattacini I agree, what I meant is all what happens under the hood. I envision a world without raw pointers, call me a dreamer :joy:

No intention to spoil the fun :joy:, but I do feel the moral obligation to remind that there was a time when YARP did not depend on std::.

Thus, a great functionality of yarp::sig::Vector was to implement a similar container. Maybe YARP 3 could be an excuse to :hocho: ~behead~ deprecate it, but a much more elegant solution with b/w compat would be preserving it, officially declaring it the vector container for efficient transport (related: #1603).

Initially Vector contained a structure supporting compatibility with GSL, but this is now gone. The other specialization is the read() and write() methods for Bottle compatibility.

Maybe a typedef would not work, but I see no reasons why deriving Vector from VectorOf should not work, and probably achieve what you want.

Consider however discussion #1603 looks like there are strong opinions towards moving containers like VectorOf in os. In addition, VectorOf could probably be now replaced by std::vector.

Whatever you do, make sure you maintain efficiency and double check Windows builds/tests, templates are difficult to export in DLLs.

Moving VectorOf in os is doable, but getting shot of it outright - because replaceable by std::vector - could be problematic considering https://github.com/robotology/icub-main/pull/503.

I wrote VectorOf replaced by std::vector, I meant replacing its implementation, but still providing the functionality to act as a portable in a YARP Port/BufferedPort.

Closed by #1657

Was this page helpful?
0 / 5 - 0 ratings