-
Notifications
You must be signed in to change notification settings - Fork 60
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
Add templated links between arbitrary datatypes #257
Merged
Merged
Changes from 78 commits
Commits
Show all changes
79 commits
Select commit
Hold shift + click to select a range
c0d22c9
Use more suitable name for header
tmadlener 9cbd240
Make things work again (again) with c++17
tmadlener aaf4b51
[wip] Start working on templated associations
tmadlener 9b94560
[wip] Almost complete implementation of Association
tmadlener 309d777
[wip] Scaffolding of AssociationCollection
tmadlener 56df83b
[wip] AssociationColllection implementation
tmadlener f9cdaed
[wip] unit tests for subset collection functionality
tmadlener aa77526
[wip] Proper move semantics
tmadlener 8b59fe9
[wip] Add necessary functionality for SIO backend
tmadlener 7bb8c21
[wip] clang-tidy fixes
tmadlener e0a3373
Move non public headers to detail folder
tmadlener de7ce7a
Fix tests after Frame
tmadlener 510561d
[wip] Make everything compile again
tmadlener 2919668
Add json output support for associations
tmadlener 79e8127
Generate all possible combinations for root dicts
tmadlener 52a9cb1
Add writing of Association collection to Frame I/O tests
tmadlener 0c344f6
Generate SIOBlocks instantiations for associations
tmadlener cc9786a
Make association read tests version dependent
tmadlener 034fc72
Fix include guards to comply with clang-tidy
tmadlener 99d47da
[wip] Make things compile again after rebase
tmadlener 9dbecea
[wip] Make associations work with Root I/O
tmadlener 942d01a
[wip] Make SIO Frame reading (almost) work again
tmadlener 4d76271
"fix" legacy sio frame test
tmadlener a1b0f3d
Make the AssociationSIOBlock public
tmadlener 6cc3c1e
[wip] Introduce macros for registering associations
tmadlener 2d3277e
Make Association I/O work with python
tmadlener c6d90ef
Avoid unnecessary string copies, add documentation
tmadlener 1b7ab3e
Add templated get/set and structured bindings to Associations
tmadlener ad77dcd
Introduce typedefs for consistency with other types
tmadlener dd29b56
Move all but the public one into detail directory
tmadlener 80ef0e8
Fix clang-tidy warning
tmadlener cd4de43
Default initialize weight to 1
tmadlener 2d6b499
Simplify and fortify SFINAE logic for mutable associations
tmadlener 9b62ef6
Add markdown documentation describing usage and some implemenation de…
tmadlener 3f6ccc9
Make things compile again after PR revival
tmadlener feb42e3
Fix some runtime errors and cleanup code
tmadlener d8c0749
Bump patch version and make all tests run
tmadlener d114191
Bring documentation up-to-date
tmadlener 8746b8d
Code cleanup and addressing review comments
tmadlener 329a784
Make sure to cleanup cloned associations properly
tmadlener 8beb807
Add functionality to be closer to Container named requirement
tmadlener 7f4bedf
Adapt version check
tmadlener d972202
Add missing header
tmadlener d3d459e
Switch to more canonical member type names
tmadlener 4a51d5d
Switch to podio docstring comment style
tmadlener 274447b
Fix cloning to behave the same as other generated types
tmadlener 6ee2ae8
Add tests for inequality operator
tmadlener 466917c
Remove unnecessary NOLINT from tests
tmadlener 90e15fc
Update type requirements to match generated code
tmadlener dd06a69
Make templated associations work the same as generated ones
tmadlener 248cbfc
Add test for cloning without relations
tmadlener d6213b7
Add definition of inequality operator
tmadlener 48f2fd2
Fix typos in documentation
tmadlener c201640
Move template helpers to class and introduce a new one
tmadlener cb155ce
Switch to different types in association for tests
tmadlener a0147af
Add test to make sure structured bindings work in loops
tmadlener c089765
Simplify and fortify some mutability checks
tmadlener bd6be25
Silence false-positives from clang-tidy
tmadlener 0719b83
Rename Association to Link after EDM4hep discussion
tmadlener 697568d
Remove empty file
tmadlener 5b75b97
Use proper preprocessor guards to avoid ROOT interference
tmadlener 903bea0
Ensure that JSON ADL is working correctly for downstream consumers
tmadlener db5a0cf
Add typeName to Interface types
tmadlener dadd7a0
Add tests for reading / writing links with interface types
tmadlener b469bf9
Make sure info for schema evolution is in buffers
tmadlener 2dbeb2e
Make sure setFrom and setTo work for python bindings
tmadlener 53d30b9
Make it possible to pass in interfaced types for setters
tmadlener 75deff4
Make LinkData an explicit type
tmadlener 5a9630f
Bump versions for tests now that a new tag has been made
tmadlener e7a55b5
Fix typo
tmadlener 2038f7d
Make initialization work for older compilers
tmadlener e81e3db
Make sure to not expect a non-generated header for roundtrip tests
tmadlener fdd1b9a
Make sure cmake runs without ENABLE_SIO=ON
tmadlener 0d1aca8
Move generic preprocessor macros to separate header
tmadlener 9a0353f
Try to reduce code duplication in macros
tmadlener e098b28
Switch to unique_ptrs for managing the related objects
tmadlener f6a6f63
Fix a few minor typos in documentation
tmadlener 1c8061e
Fix typos in comments and docstrings
tmadlener 5329877
Fix comment
tmadlener File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,224 @@ | ||
# Linking unrelated objects with each other | ||
Sometimes it is necessary to build links between objects whose datatypes are | ||
not related via a `OneToOneRelation` or a `OneToManyRelation`. These *external | ||
relations* are called *Links* in podio, and they are implemented as a | ||
templated version of the code that would be generated by the following yaml | ||
snippet (in this case between generic `FromT` and `ToT` datatypes): | ||
|
||
```yaml | ||
Link: | ||
Description: "A weighted link between a FromT and a ToT" | ||
Author: "P. O. Dio" | ||
Members: | ||
- float weight // the weight of the link | ||
OneToOneRelations: | ||
- FromT from // reference to the FromT | ||
- ToT to // reference to the ToT | ||
``` | ||
|
||
## `Link` basics | ||
`Link`s are implemented as templated classes forming a similar structure | ||
as other podio generated classes, with several layers of which users only ever | ||
interact with the *User layer*. This layer has the following basic classes | ||
```cpp | ||
/// The collection class that forms the basis of I/O and also is the main entry point | ||
template<typename FromT, typename ToT> | ||
class LinkCollection; | ||
|
||
/// The default (immutable) class that one gets after reading a collection | ||
template<typename FromT, typename ToT> | ||
class Link; | ||
|
||
/// The mutable class for creating links before writing them | ||
template<typename FromT, typename ToT> | ||
class MutableLink; | ||
``` | ||
|
||
Although the names of the template parameters, `FromT` and `ToT` imply a | ||
direction of the link, from a technical point of view nothing actually | ||
enforces this direction, unless `FromT` and `ToT` are both of the same type. | ||
Hence, links can effectively be treated as bi-directional, and one | ||
combination of `FromT` and `ToT` should be enough for all use cases (see also | ||
the [usage section](#how-to-use-links)). | ||
|
||
For a more detailed explanation of the internals and the actual implementation | ||
see [the implementation details](#implementation-details). | ||
|
||
## How to use `Link`s | ||
Using `Link`s is quite simple. In line with other datatypes that are | ||
generated by podio all the functionality can be gained by including the | ||
corresponding `Collection` header. After that it is generally recommended to | ||
introduce a type alias for easier usage. **As a general rule `Links` need | ||
to be declared with the default (immutable) types.** Trying to instantiate them | ||
with `Mutable` types will result in a compilation error. | ||
|
||
```cpp | ||
#include "podio/LinkCollection.h" | ||
|
||
#include "edm4hep/MCParticleCollection.h" | ||
#include "edm4hep/ReconstructedParticleCollection.h" | ||
|
||
// declare a new link type | ||
using MCRecoParticleLinkCollection = podio::LinkCollection<edm4hep::MCParticle, | ||
edm4hep::ReconstructedParticle>; | ||
``` | ||
|
||
This can now be used exactly as any other podio generated collection, i.e. | ||
```cpp | ||
edm4hep::MCParticle mcParticle{}; | ||
edm4hep::ReconstructedParticle recoParticle{}; | ||
|
||
auto mcRecoLinks = MCRecoParticleLinkCollection{}; | ||
auto link = mcRecoLinks.create(); // create an link; | ||
link.setFrom(mcParticle); | ||
link.setTo(recoParticle); | ||
link.setWeight(1.0); // This is also the default value! | ||
``` | ||
|
||
and similar for getting the linked objects | ||
```cpp | ||
auto mcP = link.getFrom(); | ||
auto recoP = link.getTo(); | ||
auto weight = link.getWeight(); | ||
``` | ||
|
||
In the above examples the `From` and `To` in the method names imply a direction, | ||
but it is also possible to use a templated `get` and `set` method to retrieve | ||
the linked objects via their type: | ||
|
||
```cpp | ||
link.set(mcParticle); | ||
link.set(recoParticle); | ||
|
||
auto mcP = link.get<edm4hep::MCParticle>(); | ||
auto recoP = link.get<edm4hep::ReconstructedParticle>(); | ||
auto weight = link.getWeight(); | ||
``` | ||
|
||
It is also possible to access the elements of a link via an index based | ||
`get` (similar to `std::tuple`). In this case `0` corresponds to `getFrom`, `1` | ||
corresponds to `getTo` and `2` corresponds to the weight. The main purpose of | ||
this feature is to enable structured bindings: | ||
|
||
```cpp | ||
const auto& [mcP, recoP, weight] = link; | ||
``` | ||
|
||
The above three examples are three equivalent ways of retrieving the same things | ||
from an `Link`. **The templated `get` and `set` methods are only available | ||
if `FromT` and `ToT` are not the same type** and will lead to a compilation | ||
error otherwise. | ||
|
||
### Enabling I/O capabilities for `Link`s | ||
|
||
`Link`s do not have I/O support out of the box. This has to be enabled via | ||
the `PODIO_DECLARE_LINK` macro (defined in the `LinkCollection.h` | ||
header). If you simply want to be able to read / write `Link`s in a | ||
standalone executable, it is enough to use this macro somewhere in the | ||
executable, e.g. to enable I/O capabilities for the `MCRecoParticleLink`s | ||
used above this would look like: | ||
|
||
```cpp | ||
PODIO_DECLARE_LINK(edm4hep::MCParticle, edm4hep::ReconstructedParticle) | ||
``` | ||
|
||
The macro will also enable SIO support if the `PODIO_ENABLE_SIO=1` is passed to | ||
the compiler. This is done by default when linking against the | ||
`podio::podioSioIO` library in CMake. | ||
|
||
For enabling I/O support for shared datamodel libraries, it is necessary to have | ||
all the necessary combinations of types declared via `PODIO_DECLARE_LINK` | ||
and have that compiled into the library. This is necessary if you want to use | ||
the python bindings, since they rely on dynamically loading the datamodel | ||
libraries. | ||
|
||
## Implementation details | ||
|
||
In order to give a slightly easier entry to the details of the implementation | ||
and also to make it easier to find where things in the generated documentation, | ||
we give a brief description of the main ideas and design choices here. With | ||
those it should be possible to dive deeper if necessary or to understand the | ||
template structure that is visible in the documentation, but should be fairly | ||
invisible in usage. We will focus mainly on the user facing classes, as those | ||
deal with the most complexity, the underlying layers are more or less what could | ||
be obtained by generating them via the yaml snippet above and sprinkling some | ||
`<FromT, ToT>` templates where necessary. | ||
|
||
### File structure | ||
|
||
The user facing `"podio/LinkCollection.h"` header essentially just | ||
defines the `PODIO_DECLARE_LINK` macro (depending on whether SIO support | ||
is desired and possible or not). All the actual implementation is done in the | ||
following files: | ||
|
||
- [`"podio/detail/LinkCollectionImpl.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/LinkCollectionImpl.h): | ||
for the collection functionality | ||
- [`"podio/detail/Link.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/Link.h): | ||
for the functionality of single link | ||
- [`"podio/detail/LinkCollectionIterator.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/LinkCollectionIterator.h): | ||
for the collection iterator functionality | ||
- [`"podio/detail/LinkObj.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/LinkObj.h): | ||
for the object layer functionality | ||
- [`"podio/detail/LinkCollectionData.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/LinkCollectionData.h): | ||
for the collection data functionality | ||
- [`"podio/detail/LinkFwd.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/LinkFwd.h): | ||
for some type helper functionality and some forward declarations that are used | ||
throughout the other headers | ||
- [`"podio/detail/LinkSIOBlock.h"`](https://github.com/AIDASoft/podio/blob/master/include/podio/detail/LinkSIOBlock.h): | ||
for defining the SIOBlocks that are necessary to use SIO | ||
|
||
As is visible from this structure, we did not introduce an `LinkData` | ||
class, since that would effectively just be a `float` wrapped inside a `struct`. | ||
|
||
### Default and `Mutable` `Link` classes | ||
|
||
A quick look into the `LinkFwd.h` header will reveal that the default and | ||
`Mutable` `Link` classes are in fact just partial specialization of the | ||
`LinkT` class that takes a `bool Mutable` as third template argument. The | ||
same approach is also followed by the `LinkCollectionIterator`s: | ||
|
||
```cpp | ||
template<typename FromT, typename ToT, bool Mutable> | ||
class LinkT; | ||
|
||
template <typename FromT, typename ToT> | ||
using Link = LinkT<FromT, ToT, false>; | ||
|
||
template <typename FromT, typename ToT> | ||
using MutableLink = LinkT<FromT, ToT, true>; | ||
``` | ||
|
||
Throughout the implementation it is assumed that `FromT` and `ToT` always are the | ||
default handle types. This is ensured through `static_assert`s in the | ||
`LinkCollection` to make sure it can only be instantiated with those. The | ||
`GetDefaultHandleType` helper templates are used to retrieve the correct type | ||
from any `FromT` regardless of whether it is a mutable or a default handle type | ||
With this in mind, effectively all mutating operations on `Link`s are | ||
defined using [*SFINAE*](https://en.cppreference.com/w/cpp/language/sfinae) | ||
using the following template structure (taking here `setFrom` as an example) | ||
|
||
```cpp | ||
template <typename FromU, | ||
typename = std::enable_if_t<Mutable && | ||
std::is_same_v<detail::GetDefaultHandleType<FromU>, FromT>>> | ||
void setFrom(FromU value); | ||
``` | ||
|
||
This is a SFINAE friendly way to ensure that this definition is only viable if | ||
the following conditions are met | ||
- The object this method is called on has to be `Mutable`. (first part inside the `std::enable_if`) | ||
- The passed in `value` is either a `Mutable` or default class of type `FromT`. (second part inside the `std::enable_if`) | ||
|
||
In some cases the template signature looks like this | ||
|
||
```cpp | ||
template<bool Mut = Mutable, | ||
typename = std::enable_if<Mut && Mutable>> | ||
void setWeight(float weight); | ||
``` | ||
|
||
The reason to have a defaulted `bool` template parameter here is the same as the | ||
one for having a `typename FromU` template parameter above: SFINAE only works | ||
with deduced types. Using `Mut && Mutable` in the `std::enable_if` makes sure | ||
that users cannot bypass the immutability by specifying a template parameter | ||
themselves. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
#ifndef PODIO_LINKCOLLECTION_H | ||
#define PODIO_LINKCOLLECTION_H | ||
|
||
#include "podio/detail/LinkCollectionImpl.h" | ||
#include "podio/detail/PreprocessorMacros.h" | ||
|
||
#ifndef PODIO_ENABLE_SIO | ||
#define PODIO_ENABLE_SIO 0 | ||
#endif | ||
|
||
/// Macro for registering links at the CollectionBufferFactory by injecting the | ||
/// corresponding buffer creation function. | ||
#define PODIO_REGISTER_LINK_BUFFERFACTORY(FromT, ToT) \ | ||
const static auto PODIO_PP_CONCAT(REGISTERED_LINK_, __COUNTER__) = \ | ||
podio::detail::registerLinkCollection<FromT, ToT>(podio::detail::linkCollTypeName<FromT, ToT>()); | ||
|
||
/// Macro for registering the necessary SIOBlock for a Link with the SIOBlock factory | ||
#define PODIO_REGISTER_LINK_SIOFACTORY(FromT, ToT) \ | ||
const static auto PODIO_PP_CONCAT(LINK_SIO_BLOCK_, __COUNTER__) = podio::LinkSIOBlock<FromT, ToT>{}; | ||
|
||
#if PODIO_ENABLE_SIO && __has_include("podio/detail/LinkSIOBlock.h") | ||
#include "podio/detail/LinkSIOBlock.h" | ||
/// Main macro for declaring links. Takes care of the following things: | ||
/// - Registering the necessary buffer creation functionality with the | ||
/// CollectionBufferFactory. | ||
/// - Registering the necessary SIOBlock with the SIOBlock factory | ||
#define PODIO_DECLARE_LINK(FromT, ToT) \ | ||
PODIO_REGISTER_LINK_BUFFERFACTORY(FromT, ToT) \ | ||
PODIO_REGISTER_LINK_SIOFACTORY(FromT, ToT) | ||
#else | ||
/// Main macro for declaring links. Takes care of the following things: | ||
/// - Registering the necessary buffer creation functionality with the | ||
/// CollectionBufferFactory. | ||
#define PODIO_DECLARE_LINK(FromT, ToT) PODIO_REGISTER_LINK_BUFFERFACTORY(FromT, ToT) | ||
#endif | ||
|
||
#endif // PODIO_LINKCOLLECTION_H |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it would be better to have one header where we do such things?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about putting the
PODIO_PP_CONCAT{,_IMPL}
into a separate header, but we don't have any other uses yet, so I left them here. Should we move them toinclude/podio/detail/PreprocessorMacros.h
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that could be helpful!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.