Skip to content

Extracted reco: record the KKLine track decision as KLPath without filtering the output - #587

Merged
brownd1978 merged 2 commits into
Mu2e:mainfrom
RobMina:extracted-klpath
Oct 22, 2026
Merged

brownd1978 merged 2 commits into
Mu2e:mainfrom
RobMina:extracted-klpath

Conversation

@RobMina

@RobMina RobMina commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Motivation

Since 95956ca the Extracted reco (reco/ and recoMC/Extracted.fcl) replaces KLFilter with an always-true
FixedFilter, so track-less events are kept for CRV efficiency and tag-and-probe studies. A side effect is that the
output no longer says which events had a selected track: on KPP cosmic data most saved events have none, which makes
event-display scanning and track-based analyses slow.

Change

Both files gain a second trigger path, KLPath = RecoPath + the production KLFilter (instantiated as
KLTrackFilter). The output module still selects on RecoPath, so the saved events are unchanged; the KLPath
decision is stored in TriggerResults::Reconstruct. Readers pick their view without a second stream or dataset:

reader how to get only tracked events
EventDisplay or any art job SelectEvents : [ "Reconstruct:KLPath" ] on the analyzer or output
EventNtuple FillTriggerInfo : true, TriggerProcessName : "Reconstruct" gives a trig_KLPath branch
rec files made before this PR run @local::Reconstruction.filters.KLFilter in a path of the reading job

KLPath is built from physics.RecoPath because art requires the producers a filter depends on to be on the
filter's own path; shared modules run once per event, so the added cost is the filter itself. A job that redefines
RecoPath after including Extracted.fcl must rebuild KLPath the same way
(physics.KLPath : [ @sequence::physics.RecoPath, KLTrackFilter ]).

Validation

KPP run 124155 (trk.vst stream), 5000 events of raw.mu2e.trk.vst.124155_000001.art, uncalibrated KPP data reco
(reco_data_vst.fcl on top of Extracted.fcl), Offline nightly 813ec61e0, EventNtuple 3f5abca5. The same digis
were reconstructed with this branch and with KLPath erased (= current configuration):

  • both rec files hold the same 5000 events; all 138 EventNtuple branches are identical;
  • TrigReport: RecoPath 5000/5000, KLPath 52/5000;
  • SelectEvents : [ "Reconstruct:KLPath" ], trig_KLPath, and a read-time KLFilter (on both files, including the
    one without KLPath) select the same 52 event IDs.

recoMC/Extracted.fcl was checked at configuration level only: it parses, and its KLPath is its RecoPath
(including the MC truth-matching modules) plus KLTrackFilter.

Note on the cut values

KLPath uses the production KLFilter cuts unchanged. On this uncalibrated sample 16% of events have a KKLine
track but 1% pass: minNStrawHits : 15 removes most tracks (median 10 active hits). A use case that wants a
looser selection can add its own path in the same way.

🤖 Generated with Claude Code

@FNALbuild

Copy link
Copy Markdown
Collaborator

Hi @RobMina,
You have proposed changes to files in these packages:

  • JobConfig

which require these tests: build.

@Mu2e/write, @Mu2e/fnalbuild-users have access to CI actions on main.

⌛ The following tests have been triggered for 2883075: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 2883075.

Test Result Details
test with ✅ Command did not list any other PRs to include
merge ✅ Merged 2883075 at ac5d9c9
build (prof) ✅ Log file. Build time: 04 min 28 sec
ceSimReco ✅ Log file.
g4test_03MT ✅ Log file.
transportOnly ✅ Log file.
POT ✅ Log file.
g4study ✅ Log file.
cosmicSimReco ✅ Log file.
cosmicOffSpill ✅ Log file.
ceSteps ✅ Log file.
ceDigi ✅ Log file.
muDauSteps ✅ Log file.
ceMix ✅ Log file.
rootOverlaps ✅ Log file.
g4surfaceCheck ✅ Log file.
FIXME, TODO ✅ TODO (0) FIXME (0) in 0 files
clang-tidy ✅ 0 errors 0 warnings

N.B. These results were obtained from a build of this Pull Request at 2883075 after being merged into the base branch at ac5d9c9.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@oksuzian oksuzian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review Summary — #587

Reviewed at head 288307561d63e6c65afe030dc7c80fcbacaba44b. first pass

Decision

  • 🔴 request changes. The change does what it says for both Production configs, and recording the track decision instead of filtering is a nice way to get both views from one file. But three PassN tracker jobs on PassN main include reco/Extracted.fcl or recoMC/Extracted.fcl and then redefine RecoPath, and with this PR art rejects their schedule at startup (finding 1). The fix is small, in either repository.

Scope understood

  • reco/Extracted.fcl and recoMC/Extracted.fcl add KLTrackFilter (the production KLFilter cuts, reading KKLine) and a trigger path KLPath : [ @sequence::physics.RecoPath, KLTrackFilter ]. The output and SelectReco still select on RecoPath, so the saved events do not change; the KLPath decision is stored in TriggerResults::Reconstruct.

Findings

  1. 🟠 [S1] PassN Tracker/TrkRecoFromDigi.fcl, TrkRecoFromDigiMC.fcl and TrkRecoFromFragments.fcl fail at startup with exit 9.
    • Evidence: each includes an Extracted.fcl and then sets its own, tracker-only physics.RecoPath (makeSH, SimpleTimeCluster, CosmicShowerFilter, LineFinder, KKLine, KLFilter, SelectReco, plus makeSD/EWMProducer for the fragments job). KLPath was already expanded from the full Extracted RecoPath, so it keeps CaloClusterMaker, CrvCoincidenceClusterFinder and the other calo/CRV modules, and trigger_paths becomes [RecoPath, KLPath]. KKLine and SelectReco then sit on both paths with different upstream modules, and art refuses the schedule:
      Module KKLine on paths KLPath, RecoPath depends on
        Module CaloClusterMaker on path KLPath
      Module SelectReco on paths KLPath, RecoPath depends on
        Module CaloClusterMaker on path KLPath
        Module CrvCoincidenceClusterFinder on path KLPath
      Art has completed and will exit with status 9.
      
      With Production at this head, all three exit 9 with that error; with Production main (ac5d9c91), all three build their schedule (rc 0). PassN at main 037b923, Offline and trig-config from CI backing bb141c56_ac5d9c91_32aec90c.
    • This is the case the description warns about ("A job that redefines RecoPath after including Extracted.fcl must rebuild KLPath"), but it already exists on PassN main, and there is no PassN companion.
    • Impact: the standalone PassN tracker reconstruction (TrkRecoFromDigi* and TrkRecoFromFragments, and TrkSHDFrom*, which include them) stops starting once this merges. No prodtools campaign json runs a PassN fcl, so production campaigns are not affected. Combined_Pass1.fcl is not affected either: it includes TrkRecoFromFragments.fcl but sets its own trigger_paths without KLPath, so KLPath is defined but never scheduled.
    • Suggested fix, either of:
      • a PassN companion that rebuilds physics.KLPath : [ @sequence::physics.RecoPath, KLTrackFilter ] after each physics.RecoPath redefinition in those three files, merged together with this PR; or
      • here, keep KLPath out of trigger_paths in Extracted.fcl itself and add it only in the job files that want it, so an includer that rebuilds RecoPath is not affected.

🟢 Verified at this head:

  • Production configs change only as intended. fhicl-dump of reco/Extracted.fcl and recoMC/Extracted.fcl at this head vs Production main: the only differences are KLPath, the KLTrackFilter table and trigger_paths : [ "RecoPath", "KLPath" ]. The output modules and SelectReco still have SelectEvents : [ "RecoPath" ], end_paths is unchanged (EndPath), and keep art::TriggerResults_*_*_* with process_name : Reconstruct means Reconstruct:KLPath is written.
  • The filter is in the right place and reads the right collection. KLTrackFilter is @local::Reconstruction.filters.KLFilter, a KalSeedFilter on [ "KKLine" ], and it comes after KKLine on KLPath. The shared producers run once per event, so the only added work is the filter.
  • The schedule is accepted for every Production includer. Building the schedule (0 events) gives rc 0 at this head for reco/Extracted.fcl, recoMC/Extracted.fcl and Validation/nightly/extracted_00.fcl/_03.fcl, as on main.
  • No conflict with Production#582. Both edit these two files, #582 at the top and this PR at the end. A trial merge of both onto main is clean and the combined file carries both changes.

Validation check

  • Build/tests run: FNALbuild passed at this head (merged at ac5d9c91); no buildtest job runs Extracted.fcl. Locally: fhicl-dump of both files at head and main, a schedule build of the four Production includers and five PassN fcls at head and main, and a trial merge with #582 (scripts in claude-scratch/pr587_review/: fcldump.sh, passn_dump.sh, validate_passn.sh, run_passn.sh). Two PassN fcls fail on both sides for unrelated reasons and were set aside: TrkSHDFromDigi needs StrawHitDiag, which the CI backing does not build, and Combined_Pass1 reaches endJob and fails only on geometry with 0 events. Not run: an event loop on Extracted data, since the on-disk mcs was already processed as Reconstruct and the raw/digi inputs are on /pnfs, which is not mounted here. Your 5000-event KPP comparison is the only execution evidence for the 52-event KLPath selection.
  • Config contract check: pass for Production; fail for the PassN tracker jobs (finding 1).
  • Cross-repo consistency: needs follow-up (finding 1).

Residual risk

  • Any other job outside this repository that includes Extracted.fcl and redefines RecoPath will fail the same way. I searched Production and PassN; user configs elsewhere I cannot see.

Author follow-ups

  1. Fix the PassN tracker jobs, by a companion PR or by not scheduling KLPath from Extracted.fcl itself (finding 1).

@RobMina

RobMina commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @oksuzian, finding 1 is right, and I could reproduce it. With this PR, TrkRecoFromDigi, TrkRecoFromDigiMC
and TrkRecoFromFragments on PassN main (037b923) exit 9 at schedule construction with exactly the
Module KKLine on paths KLPath, RecoPath depends on Module CaloClusterMaker on path KLPath error; with Production
without KLPath they start (rc 0).

On the two fixes: I would rather not take KLPath out of trigger_paths in Extracted.fcl. That file is the job
the campaigns run directly (e.g. data/merge_filter.json), so the decision would no longer reach the production
output, which is the point of the PR. Rebuilding KLPath in PassN is also awkward: TrkRecoFromFragments_epilog
redefines RecoPath again, and the TrkSHD* jobs cut RecoPath down to makeSH, SimpleTimeCluster with no
KKLine for the filter to read.

So I propose a two-line PassN companion instead: the tracker-only jobs opt out of the extra path right after their
RecoPath redefinition,

physics.trigger_paths : [ RecoPath ]

in Tracker/TrkRecoFromDigi.fcl and Tracker/TrkRecoFromDigiMC.fcl. TrkRecoFromFragments and the three TrkSHD*
jobs include those files and inherit it, and Combined_Pass1 already sets its own trigger_paths. Schedule build
(-n 0), Offline nightly 813ec61e0:

job Production main + PassN main #587 + PassN main #587 + PassN companion
TrkRecoFromDigi 0 9 0
TrkRecoFromDigiMC 0 9 0
TrkRecoFromFragments 0 9 0
TrkSHDFromDigi, TrkSHDFromDigiMC, TrkSHDFromFragments 9 9 9
Combined_Pass1 1 1 1

The TrkSHD* jobs fail in every column on StrawHitDiag not being built in the nightly, and Combined_Pass1 at
endJob on the unconfigured geometry with 0 events, the same two you set aside; neither reaches anything this PR
changes. The companion is Mu2e/PassN#24, to be merged together with this one.

For the residual risk (includers outside these two repos), the failure is loud and immediate (exit 9 at startup,
naming KLPath), and the fix is the same one line. fe97418 extends the comment on KLPath in both
Extracted.fcl files to name it ("after redefining RecoPath: rebuild KLPath, or set trigger_paths : [ RecoPath ]");
the processed configurations are unchanged.

@oksuzian oksuzian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review Summary — #587

Reviewed at head fe97418dcdbc0b6fc149baa466670ff675a79baf. re-review of 2883075

Decision

  • 🟡 comment only. The code is approvable: the one blocking finding from my first pass is fixed by the companion Mu2e/PassN#24. There is no FNALbuild result at this head yet, so I am holding the approval until the build is green.

Scope understood

  • The new commit only extends the comment on the KLPath line in reco/Extracted.fcl and recoMC/Extracted.fcl, naming the opt-out for jobs that redefine RecoPath. The processed configuration is unchanged.

Prior findings

  • 🟢 [was S1] PassN tracker jobs exit 9 at startup — FIXED by Mu2e/PassN#24 (head 1bef26d4), verified. That PR sets physics.trigger_paths : [ RecoPath ] right after the tracker-only RecoPath in TrkRecoFromDigi.fcl and TrkRecoFromDigiMC.fcl. Building the schedule with no events, with Production at this head:

    job PassN main (037b923) PassN#24
    TrkRecoFromDigi 9 0
    TrkRecoFromDigiMC 9 0
    TrkRecoFromFragments 9 0

    With PassN main the failure is the same cross-path error as before (Module KKLine on paths KLPath, RecoPath depends on Module CaloClusterMaker on path KLPath). With PassN#24 all three end with trigger_paths: [ "RecoPath" ]. Combined_Pass1 keeps its own four paths and no KLPath. PassN#24 also builds its schedule (rc 0) against Production main, so it can merge first, as its description says. I agree with keeping KLPath in Extracted.fcl itself: recoMC/Extracted.fcl is run directly by the mdc2025 and mdc2030 reco campaign configs, so moving the path out would lose the decision from the production output.

🟢 Verified at this head:

  • fhicl-dump of both Extracted.fcl files is identical to the first-pass head apart from comments, with trigger_paths : [ "RecoPath", "KLPath" ].
  • The four Production includers (reco/ and recoMC/Extracted.fcl, Validation/nightly/extracted_00.fcl and _03.fcl) still build their schedule (rc 0).

Validation check

  • Build/tests run: FNALbuild passed at the first-pass head 2883075; at this head mu2e/buildtest reports "This test has not been triggered yet". Locally, against Offline and trig-config from CI backing bb141c56_ac5d9c91_32aec90c: fhicl-dump of both files and a 0-event schedule build of the Production includers and the three PassN tracker jobs with PassN main and with PassN#24. Not run: an event loop, for the same input reasons as in the first pass.
  • Config contract check: pass.
  • Cross-repo consistency: pass, provided PassN#24 merges before or together with this PR.

Residual risk

  • Jobs outside Production and PassN that include Extracted.fcl and redefine RecoPath will still exit 9 at startup. The failure is loud and the new comment names the one-line fix, so I accept that.

Author follow-ups

  1. Merge Mu2e/PassN#24 first or together with this PR.

@oksuzian

Copy link
Copy Markdown
Collaborator

@FNALbuild run build test

@FNALbuild

Copy link
Copy Markdown
Collaborator

⌛ The following tests have been triggered for fe97418: build (Build queue - API unavailable)

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at fe97418.

Test Result Details
test with ✅ Command did not list any other PRs to include
merge ✅ Merged fe97418 at ac5d9c9
build (prof) ✅ Log file. Build time: 04 min 31 sec
ceSimReco ✅ Log file.
g4test_03MT ✅ Log file.
transportOnly ✅ Log file.
POT ✅ Log file.
g4study ✅ Log file.
cosmicSimReco ✅ Log file.
cosmicOffSpill ✅ Log file.
ceSteps ✅ Log file.
ceDigi ✅ Log file.
muDauSteps ✅ Log file.
ceMix ✅ Log file.
rootOverlaps ✅ Log file.
g4surfaceCheck ✅ Log file.
FIXME, TODO ✅ TODO (0) FIXME (0) in 0 files
clang-tidy ✅ 0 errors 0 warnings

N.B. These results were obtained from a build of this Pull Request at fe97418 after being merged into the base branch at ac5d9c9.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

@oksuzian oksuzian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review Summary — #587

Reviewed at head fe97418dcdbc0b6fc149baa466670ff675a79baf. Approval following my re-review at the same head.

Decision

  • 🟢 approve

FNALbuild passed at this head (build 693, merged onto main at ac5d9c91): build, ceSimReco, ceDigi, ceMix, cosmicSimReco, cosmicOffSpill, g4surfaceCheck and rootOverlaps are all green, and clang-tidy reports nothing. My earlier finding is fixed by Mu2e/PassN#24, which I have approved. Please merge PassN#24 before or together with this PR.

@brownd1978 brownd1978 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR improves the current extracted production by restoring a KL fit filtered path. It doesn't make that the default reco path.
Outside the scope of this PR, we need to refactor our reco scripts to better support all use cases, both simulation studies and efficient processing of cosmic test data. I suspect we will need multiple output streams to efficiently handle disparate cases.

@brownd1978
brownd1978 merged commit 19cf240 into Mu2e:main Oct 22, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants