feat: inherit jana::components::JOmniFactory and transition to using upstream JOmniFactory by wdconinc · Pull Request #2450 · eic/EICrecon

wdconinc · 2026-02-09T22:12:49Z

Briefly, what does this PR introduce?

This PR reapplies #1959 (reverted in #2005) and merges in #2002 for good measure. This removes our own extensive JOmniFactory support in favor of the JANA upstream JOmniFactory when version 2.4.3 is available: a thin forwarding shim class is used to allow us to keep the app argument in place for now, since it is required for < 2.4.3 versions. The only plugin factory wirings that have been modified are those where the new TypedWiring is required for variadic inputs.

The issues reported in #2000 are ultimately due to exceptions in the track reconstruction for the file used there (likely an out of sync geometry). These exceptions are gracefully absorbed in pre-2.4.3 local-JOmniFactory, but not anymore when using the upstream JOmniFactory. To make them less lethal, the exceptions are caught in the algorithm now. This shifts geometry mismatch from exceptional error to logic error (but likely a noisy one).

Needs:

What kind of change does this PR introduce?

Bug fix (issue #__)
New feature (issue #__)
Documentation update
Other: __

Please check if this PR fulfills the following:

Tests for the changes have been added
Documentation has been added / updated
Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No.

Does this PR change default behavior?

No.

…upstream JOmniFactory (fix: iwyu) (#2453) This PR applies the include-what-you-use fixes as suggested by https://github.com/eic/EICrecon/actions/runs/21885279736. Please merge this PR into the branch `jana2-2.4.3-jomnifactory` to resolve failures in PR #2450. Auto-generated by [create-pull-request][1] [1]: https://github.com/peter-evans/create-pull-request --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Wouter Deconinck <wdconinc@gmail.com>

### Briefly, what does this PR introduce? This PR (cherry-picked from #2450) adds a try-catch around cellID position lookup in the TrackerHitReconstruction.cc and LGADHitCalibration.cc. This helps build resiliency when running EICrecon against a simulation file created with a different geometry than that used in reconstruction (see e.g. #2000 which was ultimately caused by this, along with stricter treatment of exceptions in JANA2 factories used there). This shifts geometry mismatch from exceptional error to logic error (but likely a noisy one). One could argue that geometry mismatches should be treated as exceptional, but that is more true if we can do this on `init` instead of during `process`. ### What kind of change does this PR introduce? - [x] Bug fix (issue: don't treat cellID lookup failures as exceptional) - [ ] New feature (issue #__) - [ ] Documentation update - [ ] Other: __ ### Please check if this PR fulfills the following: - [ ] Tests for the changes have been added - [ ] Documentation has been added / updated - [ ] Changes have been communicated to collaborators ### Does this PR introduce breaking changes? What changes might users need to make to their code? No. ### Does this PR change default behavior? No. --------- Co-authored-by: epic-capybara <139920704+epic-capybara@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

…upstream JOmniFactory (fix: iwyu) (#2453) This PR applies the include-what-you-use fixes as suggested by https://github.com/eic/EICrecon/actions/runs/21885279736. Please merge this PR into the branch `jana2-2.4.3-jomnifactory` to resolve failures in PR #2450. Auto-generated by [create-pull-request][1] [1]: https://github.com/peter-evans/create-pull-request --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Wouter Deconinck <wdconinc@gmail.com>

Copilot

Pull request overview

This PR transitions EICrecon’s local JOmniFactory/JOmniFactoryGeneratorT implementation to thin shims over JANA’s upstream jana::components::JOmniFactory{GeneratorT}, updating plugin wirings where upstream “TypedWiring” is required (notably for variadic inputs/outputs) and adding a configure-time check to detect a known JANA 2.4.3 Input<T> bug.

Changes:

Replace the in-repo JOmniFactoryGeneratorT implementation with a forwarding shim to upstream JANA, and update multiple plugins to use upstream-style TypedWiring for variadics.
Replace the in-repo JOmniFactory implementation with a shim over upstream JANA, retaining EICrecon logging/MDC behavior and adapting input/output helper types across JANA versions.
Add a CMake configure-time probe program for a known JANA 2.4.3 JHasInputs::Input<T> bug and bump the eic-shell CI release version.

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/global/tracking/tracking.cc	Updates tracking plugin factory wiring to TypedWiring for variadic collectors/splitters.
src/global/reco/reco.cc	Updates reco plugin variadic collectors to TypedWiring and keeps shim generator usage.
src/global/pid/pid.cc	Adds includes/aliasing for upstream TypedWiring usage consistency.
src/global/pid_lut/pid_lut.cc	Migrates several variadic collectors to TypedWiring form.
src/global/particle_flow/particle_flow.cc	Adds includes/aliasing for upstream generator compatibility.
src/global/beam/beam.cc	Migrates splitter/collector wiring to TypedWiring and trims unused includes.
src/factories/tracking/LGADHitClustering_factory.h	Removes no-op `ChangeRun` override in an omni factory.
src/factories/reco/ReconstructedElectrons_factory.h	Adjusts PodioInput default naming to align with upstream wiring behavior.
src/factories/reco/LGADHitCalibration_factory.h	Removes no-op `ChangeRun` override in an omni factory.
src/factories/reco/ChargedReconstructedParticleSelector_factory.h	Adjusts PodioInput default naming to align with upstream wiring behavior.
src/factories/reco/ChargedMCParticleSelector_factory.h	Adjusts PodioInput default naming to align with upstream wiring behavior.
src/factories/meta/CollectionCollector_factory.h	Updates variadic input helper construction and adds explicit null-check behavior.
src/factories/digi/CFDROCDigitization_factory.h	Removes no-op `ChangeRun` override in an omni factory.
src/extensions/jana/JOmniFactoryGeneratorT.h	Replaces local generator implementation with a shim over upstream `JOmniFactoryGeneratorT`.
src/extensions/jana/JOmniFactory.h	Replaces local omni factory implementation with a shim over upstream `JOmniFactory`, preserving logging/MDC integration.
src/detectors/ZDC/ZDC.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/RPOTS/RPOTS.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/PFRICH/PFRICH.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/MPGD/MPGD.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/LUMISPECCAL/LUMISPECCAL.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/LOWQ2/LOWQ2.cc	Migrates multiple variadic wirings to TypedWiring and drops now-unneeded app arguments.
src/detectors/FOFFMTRK/FOFFMTRK.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/FHCAL/FHCAL.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/FEMC/FEMC.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/EHCAL/EHCAL.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/EEMC/EEMC.cc	Migrates ONNX inference wiring to TypedWiring and adds includes/aliasing.
src/detectors/ECTRK/ECTRK.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/ECTOF/ECTOF.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/DRICH/DRICH.cc	Migrates variadic merges to TypedWiring and adds includes/aliasing.
src/detectors/DIRC/DIRC.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/BVTX/BVTX.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/BTRK/BTRK.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/BTOF/BTOF.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/BHCAL/BHCAL.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/BEMC/BEMC.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/B0TRK/B0TRK.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
src/detectors/B0ECAL/B0ECAL.cc	Adds includes/aliasing for upstream generator/TypedWiring compatibility.
CMakeLists.txt	Adds a configure-time `try_run()` probe to detect a specific JANA 2.4.3 upstream bug and fail early.
cmake/test_jana243_f18b66c6.cpp	Adds the probe program used by CMake to detect the JANA 2.4.3 `Input<T>` issue.
.github/workflows/linux-eic-shell.yml	Bumps the eic-shell CI release version used in one matrix entry.

Comments suppressed due to low confidence (1)

src/global/tracking/tracking.cc:88

The new CentralTrackingRawHitAssociations wiring uses 'TOFBarrelRawHitAssociations'/'TOFEndcapRawHitAssociations', but the TOF digi chain outputs 'TOFBarrelSharedRawHitAssociations'/'TOFEndcapSharedRawHitAssociations'. This typo means TOF associations won't be merged even when available.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Wouter Deconinck <wdconinc@gmail.com>

for more information, see https://pre-commit.ci

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Removed conditional compilation for EICRECON_JANA_IS_243 and related code.

This reverts commit e1be4f7.

veprbl

Looks solid, thank you!

wdconinc · 2026-07-24T20:38:22Z

Fails for tracking_only in https://eicrecon.epic-eic.org/pr/2450/capybara/rec_dis_10x100_minQ2=1000_craterlake_tracking_only/index.html#ReconstructedParticles

Copilot

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

wdconinc force-pushed the jana2-2.4.3-jomnifactory branch 3 times, most recently from 7591981 to cf78f5d Compare February 10, 2026 22:43

github-actions Bot added the topic: digitization label Feb 10, 2026

epic-capybara mentioned this pull request Feb 10, 2026

feat: inherit jana::components::JOmniFactory and transition to using upstream JOmniFactory (fix: iwyu) #2453

Merged

wdconinc mentioned this pull request Feb 11, 2026

fix: try-catch on cellID position lookup in TrackerHitReco/LGAD #2458

Merged

7 tasks

wdconinc force-pushed the jana2-2.4.3-jomnifactory branch from 9811bd6 to 4d89276 Compare February 14, 2026 03:45

wdconinc force-pushed the jana2-2.4.3-jomnifactory branch from 4d89276 to 5d773c3 Compare July 21, 2026 18:18

wdconinc force-pushed the jana2-2.4.3-jomnifactory branch from 5d773c3 to 4350d0b Compare July 21, 2026 18:29

wdconinc force-pushed the jana2-2.4.3-jomnifactory branch from 4350d0b to 9372ac8 Compare July 21, 2026 19:07

wdconinc force-pushed the jana2-2.4.3-jomnifactory branch from 9372ac8 to 8edf9ca Compare July 21, 2026 23:24

wdconinc force-pushed the jana2-2.4.3-jomnifactory branch from 8edf9ca to 28b4276 Compare July 21, 2026 23:41

Copilot started reviewing on behalf of wdconinc July 24, 2026 01:54 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

Comment thread src/global/tracking/tracking.cc Outdated

Comment thread src/global/tracking/tracking.cc Outdated

Comment thread src/extensions/jana/JOmniFactory.h

Comment thread CMakeLists.txt

wdconinc commented Jul 24, 2026

View reviewed changes

Comment thread src/global/tracking/tracking.cc Outdated

wdconinc commented Jul 24, 2026

View reviewed changes

Comment thread src/global/tracking/tracking.cc Outdated

fix: keep TOF out of tracking

0739bfb

Co-authored-by: Wouter Deconinck <wdconinc@gmail.com>

Copilot AI review requested due to automatic review settings July 24, 2026 13:19

[pre-commit.ci] auto fixes from pre-commit.com hooks

3c63170

for more information, see https://pre-commit.ci

Copilot started reviewing on behalf of wdconinc July 24, 2026 13:19 View session

fix: retain B0 tag name

cc16f40

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

wdconinc removed the request for review from Copilot July 24, 2026 13:19

fix: always JANA2 2.4.3+ now

e1be4f7

Removed conditional compilation for EICRECON_JANA_IS_243 and related code.

wdconinc requested review from Copilot and removed request for Copilot July 24, 2026 13:24

Copilot started reviewing on behalf of wdconinc July 24, 2026 13:24 View session

Revert "fix: always JANA2 2.4.3+ now"

994d5f8

This reverts commit e1be4f7.

wdconinc requested review from Copilot and removed request for Copilot July 24, 2026 15:09

Copilot started reviewing on behalf of wdconinc July 24, 2026 15:10 View session

wdconinc requested review from a team and veprbl July 24, 2026 15:26

wdconinc enabled auto-merge July 24, 2026 16:27

wdconinc temporarily deployed to github-pages July 24, 2026 16:30 — with GitHub Actions Inactive

wdconinc added this to ePIC Review Board (auto-populated; changes will be discarded) Jul 24, 2026

veprbl approved these changes Jul 24, 2026

View reviewed changes

wdconinc moved this to Approved in ePIC Review Board (auto-populated; changes will be discarded) Jul 24, 2026

Merge branch 'main' into jana2-2.4.3-jomnifactory

14ec374

Copilot AI review requested due to automatic review settings July 28, 2026 22:09

Copilot AI reviewed Jul 28, 2026

ruse-traveler temporarily deployed to github-pages July 28, 2026 23:34 — with GitHub Actions Inactive

Navigation Menu

Uh oh!

feat: inherit jana::components::JOmniFactory and transition to using upstream JOmniFactory - #2450

feat: inherit jana::components::JOmniFactory and transition to using upstream JOmniFactory#2450
wdconinc wants to merge 18 commits into
mainfrom
jana2-2.4.3-jomnifactory

wdconinc commented Feb 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

veprbl left a comment

Uh oh!

wdconinc commented Jul 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

wdconinc commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Briefly, what does this PR introduce?

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

Does this PR introduce breaking changes? What changes might users need to make to their code?

Does this PR change default behavior?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

veprbl left a comment

Choose a reason for hiding this comment

Uh oh!

wdconinc commented Jul 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wdconinc commented Feb 9, 2026 •

edited

Loading