First version #1

S-Dafarra · 2020-08-26T16:29:12Z

It adds the skeleton, a basic interface to a generic variable, and the interface to a Vector.

What is missing:

CI
README
~~Documentation~~
Interface for other types of variables (multidimensional arrays, cell array, struct arrays)

I may start adding some documentation in the next few days. Any help is welcome.

…d declaration.

S-Dafarra · 2020-08-26T16:32:43Z

Notice that Span has been copied from iDynTree. I preferred to include it rather than introducing a dependency for a single header file.

S-Dafarra · 2020-08-26T16:42:20Z

Even if the PR looks huge, a lot of the code has been copied. The only files that should be reviewed are the CMakelists and the "code" files, except Span.h and its corresponding test which has been imported from iDynTree.

CMakeLists.txt

When installing the config file, the path given to OVERRIDE_MODULE_PATH is saved as relative to the install prefix.

traversaro · 2020-08-31T12:23:15Z

include/matioCpp/impl/Vector.tpp

+
+    if (other.isComplex())
+    {
+        std::cerr << "[matioCpp::Vector::fromOther] The input variable is complex, this is not." << std::endl;


This is not what?

Complex 😁

Ahhh, this in the C++ sense, that was not super clear for me.

I have opened an issue for fixing this #4

GiulioRomualdi

@S-Dafarra, that's really cool! Just two curiosities:

Does it work with matrices?
Does it handle eigen vectors/matrices? As far as I understood the vectors can be converted in span but the matrices not

GiulioRomualdi · 2020-09-02T07:46:18Z

include/matioCpp/Variable.h

+     * @param imaginaryData A void pointer to the (flattened) imaginary data. Check the documentation of initializeVariable for understanding how to obtain/interpret this vector.
+     * @return true in case the variable was correctly initialized.
+     */
+    bool initializeComplexVariable(const std::string& name, const VariableType& variableType, const ValueType& valueType, const std::vector<size_t>& dimensions, void *realData, void *imaginaryData);


With complex do you mean variables containing std::complex numbers?

Yes, matio supports the definition of complex numbers by returning separate arrays for the real and the imaginary part.

GiulioRomualdi · 2020-09-02T07:50:43Z

include/matioCpp/Variable.h

+     * @param data A void pointer to the (flattened) data.
+     * @return true in case the variable was correctly initialized.
+     */
+    bool initializeVariable(const std::string &name, const VariableType &variableType, const ValueType &valueType, const std::vector<size_t> &dimensions, void *data);


Just a curiosity, do you decide to handle the VariableType with a variable rather than a template because it simplifies the interface?

With this method, I need to call stuff in the pimpl, so it cannot be templated. For the same reason, I have that ugly void* at the end. Nevertheless, this is also the way matvar_t stores data inside. Its type is not known at compile time.

GiulioRomualdi · 2020-09-02T07:53:33Z

include/matioCpp/impl/Vector.tpp

+template<typename T>
+matioCpp::Vector<T>::Vector()
+{
+    static_assert (!std::is_same<T, bool>::value, "Vector<bool> is not supported." );


It is a pity that std::vector<bool> does not have the data() method

I have opened an issue for this #6

GiulioRomualdi · 2020-09-02T08:35:32Z

@S-Dafarra, that's really cool! Just two curiosities:

Does it work with matrices?

Does it handle eigen vectors/matrices? As far as I understood the vectors can be converted in span but the matrices not

Probably multidimensionalArray branch is the answer to both questions

S-Dafarra · 2020-09-02T08:56:35Z

@S-Dafarra, that's really cool! Just two curiosities:

Does it work with matrices?

Does it handle eigen vectors/matrices? As far as I understood the vectors can be converted in span but the matrices not

Probably multidimensionalArray branch is the answer to both questions

Yes, I am working on its implementation now. I am not considering matrices, but generic multidimensional arrays (i.e. with the number of dimensions >= 2). In principle, also vectors could be considered as multidimensional arrays, but I wanted to leverage the use of span for this set of objects.

For what concerns conversions to Eigen, I am avoiding them since I want to keep C++14 for the time being. Nevertheless, it should be pretty easy to define maps from matio-cpp objects, to eigen, provided that the number of dimensions is <=2.

GiulioRomualdi · 2020-09-02T09:12:06Z

For multidimensional array, you may have a look at mdspan

Interesting link: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0009r10.html

S-Dafarra · 2020-09-02T10:48:29Z

For multidimensional array, you may have a look at mdspan

Interesting link: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0009r10.html

Thanks, I'll take a look, even though it seems still a little far compared to span, which is also still experimental 🤔

prashanthr05 · 2020-09-18T17:18:40Z

@S-Dafarra May I still do a review of this PR?

S-Dafarra · 2020-09-18T17:25:51Z

It has already been merged, but feel free to comment if something is not clear. I can try to address the comments in another PR.

S-Dafarra added 8 commits July 20, 2020 18:11

Adding first version of skeleton.

27a32e5

Added utility to get type and variable from matio types. Added forwar…

40abddf

…d declaration.

Added copy/move operator/constructor and to/fromMatio.

8eb10df

Install the library as a shared library.

27fea47

Worked on the Vector draft. Added test on variable.

1244e4d

Updated Vector implementation.

8a02322

Added some methods to Vector and updated test.

6dd0254

Added iterators to the vector class.

849d5b8

S-Dafarra requested review from prashanthr05, GiulioRomualdi and kouroshD August 26, 2020 16:29

S-Dafarra self-assigned this Aug 26, 2020

Moved Vectro implementation to tpp file.

54bd837

S-Dafarra mentioned this pull request Aug 26, 2020

Create a logging tool using matio ami-iit/bipedal-locomotion-framework#69

Closed

Renamed Utilities.cpp file.

4a4515d

Added some documentation.

e343321

traversaro reviewed Aug 27, 2020

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

S-Dafarra added 4 commits August 27, 2020 14:57

Added documentation to Vector.

42853a9

The paths to look for the dependencies config files are set explicitly.

8179de6

Override paths in a relative way.

0d813e8

When installing the config file, the path given to OVERRIDE_MODULE_PATH is saved as relative to the install prefix.

Check that the input variable is not complex in Vector.

06b3159

traversaro reviewed Aug 31, 2020

View reviewed changes

GiulioRomualdi approved these changes Sep 2, 2020

View reviewed changes

S-Dafarra merged commit bc33ca3 into master Sep 2, 2020

S-Dafarra deleted the firstVersion branch September 7, 2020 09:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First version #1

First version #1

S-Dafarra commented Aug 26, 2020 •

edited

Loading

S-Dafarra commented Aug 26, 2020

S-Dafarra commented Aug 26, 2020 •

edited

Loading

traversaro Aug 31, 2020

S-Dafarra Aug 31, 2020

traversaro Aug 31, 2020

S-Dafarra Sep 2, 2020

GiulioRomualdi left a comment •

edited

Loading

GiulioRomualdi Sep 2, 2020

S-Dafarra Sep 2, 2020

GiulioRomualdi Sep 2, 2020

S-Dafarra Sep 2, 2020

GiulioRomualdi Sep 2, 2020

S-Dafarra Sep 2, 2020

GiulioRomualdi commented Sep 2, 2020

S-Dafarra commented Sep 2, 2020

GiulioRomualdi commented Sep 2, 2020

S-Dafarra commented Sep 2, 2020

prashanthr05 commented Sep 18, 2020

S-Dafarra commented Sep 18, 2020

First version #1

First version #1

Conversation

S-Dafarra commented Aug 26, 2020 • edited Loading

S-Dafarra commented Aug 26, 2020

S-Dafarra commented Aug 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GiulioRomualdi left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GiulioRomualdi commented Sep 2, 2020

S-Dafarra commented Sep 2, 2020

GiulioRomualdi commented Sep 2, 2020

S-Dafarra commented Sep 2, 2020

prashanthr05 commented Sep 18, 2020

S-Dafarra commented Sep 18, 2020

S-Dafarra commented Aug 26, 2020 •

edited

Loading

S-Dafarra commented Aug 26, 2020 •

edited

Loading

GiulioRomualdi left a comment •

edited

Loading