JeffersonLab / JANA2

Multi-threaded HENP Event Reconstruction
https://jeffersonlab.github.io/JANA2/
Other
6 stars 9 forks source link

Refactoring: Storage of Podio data #365

Open nathanwbrei opened 1 month ago

veprbl commented 1 month ago

Would it be possible to get rid of need for the visitor https://github.com/eic/EICrecon/blob/ae5ce863c38e1f7bc1ce7de414f4a3c96f0867dc/src/services/io/podio/JEventSourcePODIO.cc#L48 ? From what I understand, this is only used to populate vector<PodioT> data, which we don't even want to use.

nathanwbrei commented 1 month ago

Would it be possible to get rid of need for the visitor https://github.com/eic/EICrecon/blob/ae5ce863c38e1f7bc1ce7de414f4a3c96f0867dc/src/services/io/podio/JEventSourcePODIO.cc#L48 ? From what I understand, this is only used to populate vector<PodioT> data, which we don't even want to use.

I was thinking something similar. When I redid the example Podio file reader (https://github.com/JeffersonLab/JANA2/blob/master/src/examples/PodioFileReader/PodioFileReader.cc) I replaced the visitor with a much more obvious chain of if statements. We could probably do the same thing for EICrecon's JEventSourcePodio if we wanted.

nathanwbrei commented 1 month ago

I'm guessing you're hoping for an untyped JEvent::InsertCollectionAlreadyInFrame(const podio::CollectionBase* collection, std::string name) so we can skip the if-statement chain completely. What we have right now doesn't quite go that far, but I think it's pretty doable once this PR merges.

veprbl commented 1 month ago

I was thinking something similar. When I redid the example Podio file reader (https://github.com/JeffersonLab/JANA2/blob/master/src/examples/PodioFileReader/PodioFileReader.cc) I replaced the visitor with a much more obvious chain of if statements. We could probably do the same thing for EICrecon's JEventSourcePodio if we wanted.

Right, but the real issue is the presence of the codegen due to reliance on RTTI. Other than being (possibly) unnecessary, that makes it impossible to use other PODIO models, that are not seen by codegen.