search

JeffersonLab / JANA2

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

Remaining PODIO integration problems #247

Closed nathanwbrei closed 9 months ago

nathanwbrei commented 11 months ago

There's a bunch of PODIO-related issues which may or may not have been fixed. These are smaller complaints that pop up on comment threads and might otherwise fall through the cracks. None of them presumably affect EICrecon, but we still want our implementation to be solid.

  1. Set<>() doesn't check if the collection is already in the frame, and relies on PODIO to throw an exception with a sensible error message. This risks leaving the JFactory in a messed-up state. Usually this isn't a problem because exceptions kill the whole JANA run(), but it might become a problem if the user catches and swallows exceptions inside their JFactory. (Which, to be clear, they really shouldn't, but we have to assume someone out there will do that and have a good(ish) reason to.

  2. Insert() forgets to set mData. Ideally we'd get rid of Insert() entirely, but that's probably too much of a breaking change.

https://github.com/JeffersonLab/JANA2/pull/243#issuecomment-1747252431

nathanwbrei commented 11 months ago

Also, as explained in a comment in EICrecon/nbrei_omnifactories: JOmniFactoryGeneratorT.h: 103: The tag set via JMultiFactory::SetTag() is automatically applied as a suffix to all of the collection names? This is definitely not the behavior we want for JOmniFactories and might not be the behavior we want anywhere. Need to research what is happening with the other JMultiFactories currently in use in EICrecon

nathanwbrei commented 11 months ago

Did JMultiFactory ever get a usable m_app and m_plugin_name? If so, we can remove the hacked version in JChainMultiFactoryT. Also, JMultifactory doesn't appear to catch and re-throw std::exceptions, which it is supposed to. Finally, multifactories appear to not be providing empty collections to podio in the case of the user not populating anything. (See comment in JOmniFactoryTests.cc.)