-
Notifications
You must be signed in to change notification settings - Fork 26
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 support for reading any number of collections with Gaudi::Functional #145
Conversation
If I understand correctly this still requires the user to state all the collections that need to be read, right? I have mentioned this in #105 but I would really like to remove that necessity, so that |
Yes I think that's not hard to do, I'm not sure if it fits this PR since it's an unrelated change? Coming back to this PR; however for when reading multiple collections I don't see a way where you don't specify in |
After thinking about this a bit more, I am not sure this is the best way to go about this. Just for my understanding the main use case for this, IIUC, is to be able have algorithms that operate on more than one input collection, right? Hence, the Would it then not be easier to have a "generic" (to whatever degree possible) algorithm that simply "merges" collections? That would also allow us to easily have a list of strings as a parameter rather than a space separated string that we have to parse into a list. This would effectively take all desired collections (of the same type) and simply create a subset collection as output. This subset collection can then be used in algorithms afterwards. It is in principle a fully "virtual collection" as it should also be easy to simply drop it from the output without breaking any of the relations / associations. Then another unrelated comment to the above. This registering of the functions with the correct types makes things work with EDM4hep, right? But what happens if someone has extended EDM4hep and now wants to use their datatypes with Gaudi::Functional? Can that be easily solved? |
For the ConformalTracking we want to give a list of collections, and treat different collections differently. |
OK. And I suppose the ConformalTracking would take a variable number of input collections and that is not easily possible to accomodate in the Gaudi::Functional approach? |
0800dfb
to
7400198
Compare
0e8a8bf
to
8baed4b
Compare
Is this still relevant after #129 has been merged? |
7400198
to
e4671f4
Compare
This is probably not going to be the way this is implemented... |
Continuation of #129
Adds support to read any number of collections for cases when how many collections are going to be read is specified at runtime in the steering files. For example, let's say we want to process all the calorimeter hits and we have several collections with calorimeter hits so with this PR all of them can be read at once and processed in the same algorithm (I don't think this is supported right now in the pre-functional k4FWCore).
The collections are specified as a list of space separated collections to
PodioInput
, likeColl1 Coll2 Coll3
, and the algorithm receives astd::map<std::string, CollType*>
mapping each collection name to the type in case something different is done based on the collection name (the type of all of them should be the same). The price to pay as in #129 for the interface without wrappers andpodio::CollectionBase
is to have a templated registering of every map-type for now.Whether #129 is merged in its current state or not, we should support this way of reading.
This PR doesn't need to be merged (another PR could be made from this branch to main) but it is done this way so the diff does not show all the functional changes.