Replace PMTs with flatbuffers #31
Conversation
|
|
||
| ## Description | ||
|
|
||
| Create a new in-tree module - `pmtf` - that will live alongside `pmt` during a |
There was a problem hiding this comment.
Will GR PMTFs will be able to be understood by outside software without the need for libpmtf? If so, and i think this would be an important objective, we should state it here. Alternatively, if libpmtf is going to be necessary it might be worth starting it as an external library from the beginning to make GR more readily integrate into larger projects.
There was a problem hiding this comment.
I guess there's no reason why they wouldn't be understood outside, though libpmtf would make it much easier to parse. Using the flatbuffers API directly while possible is fairly cumbersome. Most of libpmtf prototype is just wrappers around the flatbuffers API
|
|
||
| The flatbuffers library and flatc compiler will need to be included to compile the PMT schema definition into a generated header | ||
|
|
||
| Alternatively if we don't expect the schema to change very ofted, this can be done as a |
There was a problem hiding this comment.
Is this something we could manage similar to pybind bindings? I would not expect this to change much once it is integrated, libpmt has changed relatively little its entire life so far, but CMake would need a way to clearly alert users.
There was a problem hiding this comment.
Yeah I think that would be a safe approach (method of manual generation in-tree). What I've noticed so far with flatbuffers is that the generation using flatc can vary from version to version especially with complex data types. So we would want to ensure that the in-tree generation with an optional dependency of a controlled version of flatc.
There was a problem hiding this comment.
Yes, experience from protocol buffers probably applies to flatbuffers: never save the generated code.
There was a problem hiding this comment.
So would your recommendation be assuming that flatbuffers over a range of compatible versions will generate the necessary code? How would you suggest managing the dependency?
There was a problem hiding this comment.
The flatbuffers API seems very stable, from a quick look through their changelogs. I think the risk of using different versions of flatc is low, there won't be miles of incomprehensible code checked in, and we get any improvements with new version.
So, how about any version >= whatever is currently in various distributions (11.1+?) is allowed now. If there's an incompatible change in their API in the future, revisit and possibly bump the requirement.
We can also encourage simplest possible use of the flatbuffers API. If GR's use of flatbuffers will be wrapped in GR functionality (e.g., PMT), then the usage won't get out of hand. If we open up flatbuffers as an external message interface, then a little more guidance could be needed to keep people from using every available feature.
There was a problem hiding this comment.
Generation from flatc does vary quite a lot, but the FlatBuffer folks have strong promises around cross-compatibility. If we wrap the flatbuffers as opposed to exposing them directly, then I see little risk here allowing a range of flatbuffer-versions.
|
This is a master-branch-level decision, so decider becomes @mormj. Thus making him assignee. |
|
This idea isn't fully fleshed out - would like to continue to gather feedback and get to consensus whether it is a good idea before merging. |
Proposal to create a flatbuffers wrapped pmt-like functionality with a new API