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

Remaining PODIO integration problems #247

Closed
nathanwbrei opened this issue Oct 4, 2023 · 2 comments
Closed

Remaining PODIO integration problems #247

nathanwbrei opened this issue Oct 4, 2023 · 2 comments

Comments

@nathanwbrei
Copy link
Collaborator

nathanwbrei commented Oct 4, 2023

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.

#243 (comment)

@nathanwbrei
Copy link
Collaborator Author

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
Copy link
Collaborator Author

nathanwbrei commented Oct 12, 2023

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.)

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

No branches or pull requests

1 participant