New ODIN implementation

This is an alternative to !3065 (closed)

I'm proposing a completely new implementation of the ODIN class, where the ODIN raw bank is decoded on demand and the internal storage is just the raw bank, reducing the object size (on top of DataObject) to 40 bytes instead of the current O(100) (it's difficult to judge from the number of data members).

The new class is in a dedicated (versioned) namespace to allow for multiple independent versions.

The changes are (and cannot be) backward compatible, in particular because I replaced some enums to enum class and because I renamed the enum TriggerType to TriggerTypes for consistency with the others.

I also refactored the encoding/decoding into the ODIN constructor and a special factory for decoding old versions of the bank (currently supporting only 7 and 6, but 6 can be dropped at some point). This partially addresses the issue with the copy of the decoding code in Allen.

The only issue still to be addressed is the fact that I cannot read an old ODIN object from a ROOT file. There are a few tests failing for this reason, but AFAIR we never really stored the ODIN object to file in production, so we could drop the tests or find how to implement a custom TStreamer to address schema evolution (which would be needed only to support Run2 data).

Closes #123 (closed)

Requires Lbcom!562 (merged) Rec!2464 (merged) Phys!950 (merged) Moore!877 (merged) Allen!587 (merged) Analysis!818 (merged) Panoptes!132 (merged) Castelao!114 (closed)

Includes !3250 (merged)

mentioned in merge request Lbcom!562 (merged)

mentioned in merge request Rec!2464 (merged)

mentioned in merge request Phys!950 (merged)

mentioned in merge request !3065 (closed)

mentioned in merge request Moore!877 (merged)

mentioned in merge request Allen!587 (merged)

approved this merge request

I would strongly advocate in favour of the 'interpret the underlying, original RawBank when required' because (as mentioned above) it is much more memory efficient -- one could call it the 'zero-copy' approach to decoding. It is also suspect it will be easier to maintain in case the underlying RawBank evolves. Basically, it is the way that most embedded code models I/O registers, and that is a good model to follow for ODIN ;-)

added 1 commit

• 1eafc7a9 - Removed a duplictation of ODIN raw bank definition

Compare with previous version

After the discussion of this morning, looking for the bit of code that produce ODIN banks in Boole, I found another place where the ODIN bank layout was duplicated, see 1eafc7a9.

I would like to point out that I have been thinking about using the approach used in OnlineRunInfo, but C++ does not guarantee that a struct written that way actually matches the memory layout we need (the trick of __attribute__( ( __packed__ ) ) works today with gcc and clang, but it's not portable nor standard).

added 7 commits

• b1472d0d - Add assert on buffer size in ODIN constructor
• e84d202b - Pass version to ODIN::from_version as template argument
• da71fd3e - minor code improvement
• 35c5eb25 - Wrap encoding logic in a ODIN::to_version method
• 3d9db336 - Add support for muhltiple ODIN bank versions in encoding
• 6d30c08a - Add encoding of ODIN to version 6 bank (default in ODINEncodeTool)
• f908d2b9 - Make sure ODIN Data Object version matches internal bank version

Compare with previous version

added 1 commit

• c73676b4 - Fixed formatting

Compare with previous version

To be able to produce version 6 ODIN banks in Boole, I had to refactor the encoding (and the decoding for symmetry).

Now one can decode a raw bank with:

auto odin = LHCb::ODIN::from_version<7>( buffer );

and encode the ODIN object to a raw bank with:

std::array<unsigned int, 10> output_buffer;
odin.to_version<6>( output_buffer );

(I cannot return the output_buffer because I keep the various to_version<N> outside of ODIN.h)

I still have to

• replace gsl::span with LHCb::span (do I?)
• make the DataObject dependency #ifdef-able away
• remove the unfixable IOExample tests
• add tests for all encoding/decoding combinations

added 4 commits

• 6f833085 - Add support for multiple ODIN bank versions in encoding
• 0c2d297b - Add encoding of ODIN to version 6 bank (default in ODINEncodeTool)
• 2d339eab - Make sure ODIN Data Object version matches internal bank version
• 4bfa3778 - Remove Reco08 MDST tests

Compare with previous version

added 2 commits

• c9ef699b - Allow building ODIN.h stand-alone
• a14862ea - Add basic tests of ODIN codecs

Compare with previous version

added 1 commit

• 93faa213 - Add basic tests of ODIN codecs

Compare with previous version

added 2 commits

• e3b2bf47 - fixed a couple of tests
• 2ae0bbe9 - Use LHCb::span instead of gsl::span

Compare with previous version

changed the description

added Decoding Event model new feature labels