Skip to content

Configs to exercise CentralHelix fit and extrap in nightly validation - #580

Merged
oksuzian merged 2 commits into
Mu2e:mainfrom
RobMina:nightly-offspill-centralhelix
Oct 7, 2026
Merged

oksuzian merged 2 commits into
Mu2e:mainfrom
RobMina:nightly-offspill-centralhelix

Conversation

@RobMina

@RobMina RobMina commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Currently the nightly validation does not exercise CentralHelix track fit or extrapolation.

@FNALbuild

Copy link
Copy Markdown
Collaborator

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

  • Validation

which require these tests: build.

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

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

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 94f1a7f.

Test Result Details
test with ✅ Command did not list any other PRs to include
merge ✅ Merged 94f1a7f at 7d83b5f
build (prof) ✅ Log file. Build time: 04 min 26 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 94f1a7f after being merged into the base branch at 7d83b5f.

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 — #580

Reviewed at head 94f1a7f946976fd9ba362446a2759fc09657a013. first pass

Decision

  • 🟢 approve. Nightly validation should exercise the CentralHelix fit and its CRV extrapolation, and the config is a clean delta on JobConfig/recoMC/OffSpill.fcl. The notes below are questions, not blockers.

Scope understood

  • New Validation/offSpill.fcl. It reconstructs CosmicSignalOffSpill digis through the stock OffSpill reco and replaces CHFilter with a pass-all FixedFilter. It writes only the CentralHelix products, sending the LoopHelix output to /dev/null, for 10000 events.
  • Five Validation/nightly/offSpill_0N.fcl drivers, one MDC2025an_best_v1_1 input file each, following the extracted_0N.fcl pattern.

Verified, no action needed

  • 🟢 fhicl-dump resolves both offSpill_00.fcl and offSpill_04.fcl against this head. The dump shows CHFilter as FixedFilter with ReturnValue: true, CentralHelixOutput selecting on CHPath, the per-job output file names, maxEvents: 10000, and DbService set to Sim_best v1_5 from database.fcl.
  • 🟢 The CHFilter override does what the comment says. In JobConfig/reco/prolog.fcl:367-372, CentralHelixPath is PreTrk, OffSpillTrk, CHFilter, SelectReco. KKCHmu is fitted before the filter, so passing every event changes only what reaches SelectReco and the output, not what is fitted.
  • 🟢 The keep list mirrors Validation/reco.fcl, including the StageNormalization keep that reco.fcl documents as necessary. KalSeeds_KKCHmu, CosmicTrackSeeds_LineFinder, SimpleTimeCluster and CrvCoincidenceClusters are swapped in for the LoopHelix products.
  • 🟢 CI: the FNALbuild build test passed at this head, and all mu2e/* contexts are green.

Findings

  1. 🟡 [S2] DB version versus input dataset. The inputs are MDC2025an_best_v1_1 digis, but database.fcl reconstructs them with Sim_best v1_5.
    • That's the same pattern the existing reco_0N jobs use (MDC2025au_best_v1_3 inputs on v1_5), so it may be intended: validate current reco conditions on older digis.
    • When I reconstructed this same dataset for the KinKal#226 A/B, I used Sim_best v1_1. I haven't checked whether v1_5 changes the CRV or calo calibration seen by these files.
    • If the intent is a stable reference, say so in the comment. Otherwise, consider pinning v1_1.
    • Not run: a short job at this head (see Validation check).
  2. ⚪ [S3] LHPath still runs on every event. Its output goes to /dev/null, but the LoopHelixPath fits (KKDe etc.) are still done. Setting physics.trigger_paths : [ CHPath ] would drop that work.
    • It would also keep LHFilter out of TriggerResults.
    • Not measured. The comment's "~0.2 s/event" may already include it.
  3. ⚪ [S3] Not checked: how the nightly harness picks up these jobs. The other families with numbered drivers have a nightly/<Family>/build_jobs.sh; extracted_0N.fcl and these new files don't. I'm assuming offSpill_* is picked up the same way extracted_* is.

Validation check

  • Build/tests run: fhicl-dump of offSpill_00.fcl and offSpill_04.fcl at this head, both rc=0. I didn't run an art job; an attempt was blocked in my session. The FNALbuild build test passed.
  • Config contract: pass, from the fhicl-dump output above.
  • Cross-repo: none. CentralHelix efficiency is under discussion in KFTrack/KinKal#226, and this job is how the nightly will see it.

Residual risk

  • Low. The change is additive and touches only the nightly validation. The main open point is the DB question in finding 1.

Author follow-ups

  1. Confirm Sim_best v1_5 on MDC2025an_best_v1_1 inputs is intended (finding 1).
  2. Optional: drop LHPath from trigger_paths (finding 2).

@rlcee

rlcee commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

How does this not overlap the existing tests or can the existing test be adapted to or replaced by this test? Usually there is some discussion.

@rlcee
rlcee self-requested a review October 6, 2026 12:01
@RobMina
RobMina requested a review from brownd1978 October 6, 2026 15:38
@RobMina

RobMina commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Currently none of the existing tests run off-spill reconstruction, so they do not exercise the CentralHelix (mostly vertical cosmic muon) tracking nor the LoopHelix upstream e/mu fits.

None of the existing tests use a digi dataset with field-on and cosmic muons that intersect the CRV, so extrapolation outside to the CRV cannot be tested using the existing configurations. I plan to open a PR with additions to Offline/Validation to histogram/compare CRV intersections for this dataset.

Regarding the DB version, dbTool print-run --run 1430 gives the same calibration entries for all 20 tables in v1_1 and v1_5

Regarding turning off LHPath, CentralHelixPath and LoopHelixPath run the same reconstruction modules: preprocessing plus OffSpillTrk, which includes all four LoopHelix fits as well as the CentralHelix reco. They differ only in their output filter.

@rlcee

rlcee commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

I thought cosmicSimReco was field-on, but I see it is not, so we have two field-off cosmics. Some more options:

  • replace cosmicSimReco with offSpill
  • turn field on cosmicSimReco
  • add offSpill files to reco
    A while ago, I started an analysis of algorithm coverage in the nightly, to try to be more efficient in the set of tests, but it didn't go anywhere

@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 — #580

Reviewed at head 253dc6c9d3f59ddb16b34c4814c5c8c70e7e9005. re-review of 94f1a7f946976fd9ba362446a2759fc09657a013

Decision

  • 🟢 approve. The new commit only widens the output keep list and updates comments. The new keep patterns match products the job makes, and the prior DB question is now answered and documented.

Scope understood

  • The increment is one commit, 253dc6c9, touching only Validation/offSpill.fcl:
    • KalSeeds_KKCHmu becomes KalSeeds_KK*, and HelixSeeds_MH* and TimeClusters_MH* are added, so the single nightly file also validates the four LoopHelix fits on cosmics.
    • The DB-version comment and the timing comment are updated.

Verified, no action needed

  • 🟢 The new keep patterns match real products. I ran fhicl-dump on offSpill_00.fcl and offSpill_04.fcl at this head, and both returned rc=0. The only differences from the dumps at 94f1a7f9 are the three keep lines, plus the input file that differs between the two jobs.
    • CHPath contains MHDe/MHUe/MHDmu/MHUmu. MergeHelices (TrkReco/src/MergeHelices_module.cc:77-78) produces HelixSeedCollection and TimeClusterCollection.
    • KK* covers KKDe/KKUe/KKDmu/KKUmu/KKCHmu, which are all in CHPath. KKLine is configured but not on any path, so it adds nothing.
  • 🟢 The comment that CHPath selects every event holds. CHFilter is a FixedFilter with ReturnValue: true. My local 100-event run of offSpill_01 at 94f1a7f9 passed CHPath on 100 of 100 events, and KKDe/KKUe/KKDmu/KKUmu/KKCHmu/MHDe all ran on 100 of 100.
  • 🟢 The timing comment ("40-90 minutes") is consistent. The same run took 24.4 s CPU and 45.7 s wall for 100 events, which is about 41 to 76 minutes for 10000 events.
  • 🟢 [was S2] DB version against the inputs: FIXED in 253dc6c9, verified. I ran dbTool print-run --run 1430 for Sim_best v1_1 and v1_5. Both list the same 21 tables with the same CIDs. The only difference is the IoV end of TrkPanelStatus CID 110 (1499 versus 1439), and both IoVs cover run 1430. So the new comment, that both versions resolve to identical conditions for run 1430, is accurate.

Findings

  1. ⚪ [was S3] "LHPath still runs every fit": WITHDRAWN. art runs a module label once per event however many paths it appears on, and LHPath and CHPath share every reco module, as the author pointed out. Dropping LHPath would save only LHFilter and the /dev/null output, not any fits. My earlier finding was wrong.
  2. ⚪ [S3, carried over] Not checked: how the nightly harness picks up offSpill_0N.fcl. As with extracted_0N.fcl, there's no build_jobs.sh for this family. I'm assuming it is picked up the same way extracted_* is.

Validation check

  • Build/tests run: fhicl-dump at this head (above) and dbTool print-run for both DB versions. I didn't rerun the art job at this head, because the increment changes only the output keep list and comments.
  • CI: there is no FNALbuild build at this head yet. mu2e/buildtest reports "This test has not been triggered yet". The build at 94f1a7f9 passed.
  • Config contract: pass, from the dump above.
  • Cross-repo: pass. Mu2e/Offline#2023 adds the CRV-intersection histograms for this job's KKCHmu output, and the two can merge in either order.

Residual risk

  • Low.
  • The output now carries five KalSeed collections. I didn't measure the file size at 10000 events.
  • @rlcee's question about overlap with existing nightly jobs is a coverage decision for the maintainers, not a defect: replacing cosmicSimReco, turning the field on there, or folding off-spill into reco.

Author follow-ups

  • None required.

@oksuzian

oksuzian commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

@FNALbuild run build test

@FNALbuild

Copy link
Copy Markdown
Collaborator

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

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 253dc6c.

Test Result Details
test with ✅ Command did not list any other PRs to include
merge ✅ Merged 253dc6c at 7d83b5f
build (prof) ✅ Log file. Build time: 09 min 06 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 253dc6c after being merged into the base branch at 7d83b5f.

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 — #580

Reviewed at head 253dc6c9d3f59ddb16b34c4814c5c8c70e7e9005. Follow-up to my comment review at this same head (pullrequestreview-5434185118), which held only for a build.

Decision

  • 🟢 approve. FNALbuild's build at this head has passed ("The build tests passed at 253dc6c…"), and so has every mu2e/* validation context. My earlier review at this head stands as written: the prior DB-version finding is fixed, the LHPath finding is withdrawn, and the harness-pickup note is unchecked and does not gate.

@brownd1978

brownd1978 commented Oct 6, 2026 via email

Copy link
Copy Markdown
Collaborator

@oksuzian
oksuzian merged commit 0d1c370 into Mu2e:main Oct 7, 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.

5 participants