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

[WIP] Add a generic variant based type that can be used for interface-like functionality #215

Closed
wants to merge 22 commits into from

Conversation

tmadlener
Copy link
Collaborator

@tmadlener tmadlener commented Sep 9, 2021

BEGINRELEASENOTES

  • Add a podio::GenericWrapper class that can wrap multiple user data types via an underlying std::variant.
  • Make it possible to add an optional interface section to the datamodel definition where "interface" classes can be defined. These datatypes use the podio::GenericWrapper and can store multiple different user datatypes that all offer similar functionality and serve as a sort of "base type" that can then be used as any other datatype.

ENDRELEASENOTES

Contains all changes from #143 and #217

Rationale

It is possible that there are specialized datatypes that all offer very similar functionality, e.g. in LCIO there is a whole hierarchy of TrackerHit classes, which means that e.g. EDM4hep will also potentially need more than one TrackerHit class (see key4hep/EDM4hep#121). Adding more datatypes is quite trivial (e.g. key4hep/EDM4hep#122). However, it is currently not possible to have a heterogeneous collection of different datatypes in podio generated datamodels. Hence, e.g. a "simple" Track class, which can store different types of TrackerHits within a single OneToManyRelation is not possible and a definition would have to look like this:

edm4hep::Track:
  # all the other things
  OneToManyRelations:
    - edm4hep::TrackerHitPlane planeHits // planar tracker hits
    - edm4hep::TrackerHit                // tracker hits
    # potentially more different types

This makes for a rather awkward usage as all the possible relations have to be checked and it is not easily possible to simply loop over all tracker hits, regardless of their type. Additionally, every time a new tracker hit type is added all user code that does such a loop would have to be changed, to now also include the new type (arguably such additions would hopefully be few, but even once might be enough already)

Proposed solution

podio::GenericWrappers

The proposed implementation solves this problem by introducing a podio::GenericWrapper class, that on a very basic level looks like this (not strictly valid c++, just for illustrating the point, see the code for details)

template<bool Mutable, typename ...WrappedTypes>
class GenericWrappers {
  // some functionality
  // The internal variant that stores the Obj* of all wrapped types
  std::variant<WrappedTypesObjPtr...> m_obj;
};

Using the internal std::variant the GenericWrappers class offers some basic functionality that is expected from all user-facing objects (e.g. getObjectID), via std::visit. Additionally, it offers functionality to probe what the currently held type is, as well as functionality to try and get the "actual" type from the generic wrapper (if it is currently held). These are the two functions:

template<typename U, /*enable_if trickery*/>
bool isCurrentType() const;

template<typename U, /*enable_if trickery*/>
U getValue() const;
// + a non-const overload for cases where Mutable == true

With this class users can already do something like

using TrackHit = podio::GenericWrapper<true, edm4hep::TrackerHit, edm4hep::TrackerHitPlane>;
auto trackHit = TrackHit{edm4hep::TrackerHit()};
assert(trackHit.isCurrentType<edm4hep::TrackerHit>());

trackHit = edm4hep::TrackerHitPlane();
auto planeHit = trackHit.getValue<edm4hep::TrackerHitPlane>();

Interface types

The GenericWrapper class allows to store different tracker hit classes in one type. However, its functionality is (deliberately) limited and to do something useful it is still necessary to cast back to the original tracker hit type. To solve this, additional wrapper classes are introduced to the generated code, which inherit from podio::GenericWrapper. They are generated from datatype definitions in a new section of the datamodel yaml (taking edm4hep and LCIO here, but there is also another example in the PR for the podio example datamodel)

datatypes:
  edm4hep::TrackerHitPlane:
    # definition
  edm4hep::TrackerHitZCylinder:
    # definition

  edm4hep::Track:
    # all the rest
    OneToManyRelations:
      - edm4hep::TrackerHit trackHit // track hits. NOTE not to be confused with the current TrackerHit!
  
interfaces:
  edm4hep::TrackerHit:
    # description + author omitted
    Types:
      - edm4hep::TrackerHitPlane
      - edm4hep::TrackerHitZCylinder
     Members:
      - edm4hep::Vector3d position // the 3D position 

For all datatypes in the interfaces section user facing classes are generated that behave exactly the same as other datamodel classes, with the only difference, that they have to be initialized with another value (they cannot be default constructed). The Members section of each type defines the publicly available setter and getter functions on these "interface" types. In this case only the getPosition (and setPosition on the mutable classes) would be available from the TrackerHit class (while TrackerHitPlane would have more getters/setters). Note that in the current implementation these have to be actual data member of the wrapped types.

Some implementation details

  • All generated user facing classes have gained a public ObjPtrT typedef for the corresponding Obj*
  • The mutable user facing classes additionally have gained a public ConstT typedef for the corresponding Const class
  • The first boolean template parameter in podio::GenericWrapper steers whether a non-const getValue template function is available, that also gives access to mutable user facing classes. If it is false, only the const version will be available, that will also give access to the Const classes. This is necessary to not introduce a way to do an accidental const_cast, by initializing the wrapper with a Const value but then getting a mutable value out of it.

  • Clean up commit history
  • Document GenericWrapper
  • Naming and syntax
    • In the datamodel yaml files (do we want to call the new section interfaces?)
    • For the GenericWrapper class (I really think the current name doesn't really capture the intent, but I haven't found a better one yet)
  • Find out why two of the CI workflows are failing (and why is it only two that are failing)
  • Fix access rights again (partially released during development)

Only necessary to be non-default in case of relation handling
Necessary to use OneToOneRelations in a different namespace
Interface types can only use Types that are defined in the datatypes
section. datatypes can only use interface types as relations (same as
with other datatypes). Members of interface types are not yet checked.
Previously the getValue call made it possible to create a GenericWrapper
from a Const data type but get a mutable one out
Make the all possible GenericWrappers friends of the user facing
classes. This gives the wrappers the necessary access, while still
keeping everyone else from getting its hand on the Obj*
Also make sure that using types from different namespaces is possible in
interface types
@hegner
Copy link
Collaborator

hegner commented Jun 27, 2023

@tmadlener - would be great to actually see the foreseen user code use cases and improvements

@tmadlener
Copy link
Collaborator Author

superseded by #516

@tmadlener tmadlener closed this Nov 22, 2023
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

Successfully merging this pull request may close these issues.

2 participants