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

Consistent implementation optional input/output collections for factories #1235

Open
veprbl opened this issue Jan 15, 2024 · 1 comment
Open

Comments

@veprbl
Copy link
Member

veprbl commented Jan 15, 2024

We have a few use cases for collections that may be missing in the input files:

  • Subsystems missing in the detector configuration or otherwise excluded from simulation
  • Processing of "real data" without access to the sim hit information

Those omitted collections may trigger omissions in the output collections. We would like to be able to gracefully handle those cases while performing as much sanity checks for the collections that we expect to be present. Unfortunately, it is not even clear if we can have both at the same time.

The current code largely relies on muting exceptions to achieve this, e.g.:

catch(std::exception &e) {
// ignore missing collections, but print them in debug mode
m_log->debug(e.what());
}

and #1069

cc @nathanwbrei

@simonge
Copy link
Contributor

simonge commented Jun 28, 2024

Following on from this conversation #1503 (comment) in the related issue rather than a closed PR.

It's present in the upstreamed version of OmniFactory that lives under JANA/Omni/JOmniFactory. Both PodioInputs and VariationalPodioInputs accept an is_optional flag. If you set this flag to true, the collection pointers will be allowed to be null, so you have to check for that. I haven't updated EICrecon to use the upstreamed version of OmniFactory anywhere yet, but you could experiment with it if you like, and report back any problems you find.

Note that if you use the upstreamed JOmniFactory, you'll probably have to use the upstreamed JOmniFactoryGenerator that goes with it. Both of these live in the jana::omni namespace, so they hopefully shouldn't conflict with the EICrecon version.

I haven't been able to simply swap the jana::omni version of the factory in for an individual/subset of factories. Probably a lack of understanding on my part whether it's possible to do this for individual factories/plugins or if it has to be done across the whole codebase. My short term goal is just to convert all instances of CollectionCollector factories so that it doesn't matter if some collections aren't available.

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

2 participants