Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialization/Deserialization #19

Open
ahundt opened this issue Nov 7, 2016 · 6 comments
Open

Serialization/Deserialization #19

ahundt opened this issue Nov 7, 2016 · 6 comments

Comments

@ahundt
Copy link

ahundt commented Nov 7, 2016

Is there anything available for serialization/deserialization of robots (MultiBodies), their configurations, their constraints, etc?

@haudren
Copy link
Collaborator

haudren commented Nov 9, 2016

Unfortunately, not really... On the robot side, @jorisv had created facilities to pickle/unpickle robots from python, but we got rid of it when creating our new bindings. There should be a way to re-implement it now that the interface has stabilized. We never implemented facilities to serialize/deserialize solver-related quantities but it could also be nice, as it would allow us to properly stop and restart execution.

@ahundt
Copy link
Author

ahundt commented Nov 12, 2016

Okay, I might have use cases to implement such a feature. I'd likely create a separate header file to manage the serialization so a dependency isn't added, and I'd most likely use either google protobuf or google flatbuffer. The advantage of this is it could easily support both python and C++ out of the box.

This stackoverflow question has some links and information I've compiled on how to achieve this.

@ahundt
Copy link
Author

ahundt commented Jan 27, 2017

@gergondet @haudren I expect I might be coming back to serializing to protobufs or flatbuffers again in the next few months. I'd like to write the serialization it in a way which allows me to make a useful pull request you'd be willing to accept, so I'd appreciate the chance for some design discussion!

For instance, if either protobuf or flatbuffers was an optional CMake dependency for RBDyn, SVA, and Tasks etc, would that be acceptable?

I also imagine putting such code in an include/protobuf or include/flatbuffer folder. I'd also plan to have functions that allow the internal state to be extracted but don't add dependencies on protobuf to any headers. This could be done in various ways such as with an access API or with ifdefs. Is there any preference?

@haudren
Copy link
Collaborator

haudren commented Jan 27, 2017

I don't have a very clear idea yet but:

  • It should definitely be an optional CMake dependency/target: I don't think we'll want to install the serialization code on every platform where we use Tasks.
  • If possible, I'd like it to be another library, something like tasks-protobuf. I'm afraid that this will complicate the API though:
rbd::MultiBody mb;
mb.load('myfile');
//vs
mb = rbdProtoBuf::loadMb('myfile');
// or maybe
auto mb = rbdProtoBuf::load<rbd::Multibody>(myFile);
  • For the code itself I'd rather put it in a folder that describes the functionality, something like serialization/protobuf. It will avoid confusing the library's headers (which are in src) with the optional headers/code (i.e. what's in bindings, tests...).
  • For the protobuf/flatbuffers choice: I like the flexibility that flatbuffers has (mainly the fact that you can specify unions, and serialize to text) but it's not available in the Ubuntu repositories. I'd like to avoid adding non-packaged external dependencies as a courtesy to our users.

@gergondet
Copy link
Member

I don't think the feature has to be optional but it would depend greatly on the library you choose to go with for serialization, i.e. whether it adds an external binary dependency (e.g. protobuff) or not (e.g. flatbuffer). In any case, any serialization header should be outside of the public headers of SVA/RBDyn/Tasks. This can be easily achieved for RBDyn and Tasks but not really with SVA as this is currently a header-only library which would change in that case.

In the first case (serialization code brings an additional binary dependency) then I'd rather have a separate project (or projects) for the serialization although the result would be less pleasant to use.

If we can get away with the dependency (this is possible/recommended with FlatBuffers according to the documentation) then I wouldn't mind having it as part of the class. If the serialization library requires an IDL compilation then I think the best would be to have both the IDL and compiled header file in the repository and installed as part of the project (e.g. ${CMAKE_INSTALL_PREFIX}/include/${PROJECT_NAME}/serialization/${CLASS_NAME}.h for the compiled IDL and ${CMAKE_INSTALL_PREFIX}/share/${PROJECT_NAME}/serialization/${CLASS_NAME}.idl for IDL files) so that most people don't need to fetch a full installation of the serialization library runtime just to compile the library but giving the option to define new IDL based on the existing ones.

Between the two options, I have a strong preference for the second one.

A few must-have features (imho) of serialization, regardless of the library:

  • human-readable/editable text format support
  • backward compatible serialization format
  • platform independent binary representation (in particular 32/64 bits compatibility)
  • cross-platform

I think FlatBuffers meet these requirements and it looks like it would be the easiest to integrate into the projects. The most contentious part would be regarding the transformation of SVA from a header-only library to a compiled library. This doesn't make a huge difference in our use-cases however and we could take the opportunity to bring-in pre-compiled templates into the mix.

@ahundt
Copy link
Author

ahundt commented Jan 31, 2017

protobufs also have a text format.

Seems googlecartographer has some appropriate examples we could use as a useful reference for either protobuf or flatbuffers:
https://github.com/googlecartographer/cartographer/blob/master/cartographer/mapping/trajectory_node.cc
https://github.com/googlecartographer/cartographer/blob/master/cartographer/transform/transform.h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants