Skip to content

AgnosticHelixFinder: best-of-K candidates, 2π alias test, target-constrained refit - #2038

Open
michaelmackenzie wants to merge 6 commits into
Mu2e:mainfrom
michaelmackenzie:apr-helix-finding
Open

michaelmackenzie wants to merge 6 commits into
Mu2e:mainfrom
michaelmackenzie:apr-helix-finding

Conversation

@michaelmackenzie

Copy link
Copy Markdown
Contributor

This PR adds three opt-in AgnosticHelixFinder options, aimed at trackers with fewer hits per track, and a truth analyzer for studying APR helix finding. All new options default to the previous behavior. With the defaults, the trigger decisions and helix parameters are bit-identical to before (checked on 3000 CeM and 3000 CeP events).

The PR sits on top of #2035 (HelixTool: don't count straws without signal in the helix hit ratio), so that commit appears here until it is merged.

Motivation

With the commissioning tracker conditions (Sim_commissioning: 31% of straws dead, including all of station 7, lower gas gain, more threshold noise), the APR trigger loses most of its CE efficiency. A truth study (AprHelixTruthAna, below) shows the finder still finds a helix on the signal hits, but often with the wrong parameters. For CeM events with ≥ 15 signal straw hits:

  • wrong λ: 17% (nominal tracker: 0.6%)
  • wrong circle: 16% (nominal: 3.6%)

A CE crosses the active region once per turn, so its hits come in short groups about 2πλ apart. The half-radius / half-λ helix, or a 2π φ-z alias, fits the same hits.

Changes

CalPatRec/src/AgnosticHelixFinder_module.cc

option default description
maxHelixCandidates 1 Keep the best of up to K viable candidates per search (most straw hits, then the lowest circle + line χ²/dof) instead of the first one.
acceptHelixStrawHits 0 (off) With K > 1, save a candidate with at least this many straw hits immediately, skipping the rest of the search (saves time).
minAliasGap 0 (off) If the largest z gap between the helix hits is at least this (mm), refit the φ-z line with the hits beyond the gap shifted by ±2π. Keep the lowest χ²/dof within the momentum range, then recover points again.
targetRefitSigma 0 (off) With K > 1: if the best candidate has |d0| > this (mm), refit its circle through the stopping target (error = this) and find the φ-z line with a dφ/dz scan on that circle. The refit replaces the candidate if it has at least as many straw hits.
targetRefitPhiWindow, targetRefitDphiDzBins 0.5 rad, 100 φ window and number of slope bins (per helicity) for the refit's dφ/dz scan.

The refit replaces the best candidate instead of adding a second helix. Adding one gave the same signal efficiency, but its duplicate helices fired the multi-helix and two-track paths and added about 20% background on the APR TrkDe paths.

Analyses/src/AprHelixTruthAna_module.cc (new)

A per-event TTree that compares a helix seed collection with the MC truth of the signal particle (the highest-momentum e± or μ± with ≥ minHits straw hits):

  • the particle's truth at its first straw gas step: momentum, direction, circle, λ
  • the z span and gaps of its combo hits
  • for each helix: its parameters, straw hits and signal straw hits, and its circle χ² against the reco and the true circle

Optionally it records the trigger path decisions of the current process (triggerPaths), so trigger efficiencies can be split by the signal truth (for example particle type and direction for cosmics).

Results

Trigger re-run on MDC2025au CeM/CeP + 1BB pileup, re-digitized with Sim_commissioning. The reduced-tracker baseline uses tuned trigger cuts (in mu2e-trig-config fcl, not part of this PR). Absolute efficiency is relative to the MC-triggerable selection of the dataset; 3000 events per sample.

apr_TrkDe_75m70p_D0200 efficiency (cumulative):

step CeM CeP
baseline (reduced tracker, tuned cuts) 52.9% 28.5%
+ maxHelixCandidates 3, acceptHelixStrawHits 20 56.7% 31.4%
+ minAliasGap 100 57.5% 32.6%
+ looser φ-z cuts (existing parameters: maxPhiZResidual 3, segMultiplier 2, maxSegmentChi2 4) 60.4% 36.8%
+ targetRefitSigma 100 61.9% 39.1%
nominal tracker, previous finder → all changes 83.3% → 84.7% 67.6% → 69.3%
  • Offline reference: for CE events with a good Offline track (≥ 15 active hits), CPR OR apr_TrkDe_75m70p_D0200 reaches 95.0% (CeM) and 90.4% (CeP). With the nominal tracker these are 97.5% and 94.1%.
  • Cosmics (CosmicSignalMix1BB, p > 75 MeV/c): apr_TrkDe_75m70p goes from 34.3% to 39.6% on the reduced tracker and from 48.1% to 51.8% on the nominal tracker.
  • Background (NoPrimaryMix1BB, 200k microbunches, paired, same file grouping): apr_TrkDe_75m70p_D0200 passes 28 → 22 events. The non-D0200 APR TrkDe paths rise about 27%.
  • Timing: TrigAprHelixFinder goes from 0.036 to 0.067 ms per event, and the APR KalSeed fits from 0.19 to 0.25 ms (more helix candidates). That is about +2–4% of the main-path trigger time.

Example configuration:

physics.producers.TrigAprHelixFinder.maxHelixCandidates   : 3
physics.producers.TrigAprHelixFinder.acceptHelixStrawHits : 20
physics.producers.TrigAprHelixFinder.minAliasGap          : 100
physics.producers.TrigAprHelixFinder.targetRefitSigma     : 100

Notes for reviewers

  • Found while reading findSeedPhiLines, not changed here:

    • lastAddedPositiveZ and lastAddedNegativeZ are never updated, so maxSeedLineGap acts as a maximum distance from the seed hit rather than a gap between hits.
    • The negative line's slope test divides by positiveDeltaZ.

    Fixing both made no measurable difference to the signal efficiency in these tests, so I left them for a separate change.

  • The trigger cut tuning for the commissioning tracker (mu2e-trig-config) is a separate change.

🤖 Generated with Claude Code

michaelmackenzie and others added 3 commits October 13, 2026 14:44
The hit ratio (measured straw hits / expected plane-face crossings) counted
every face the helix crosses inside the tracker active radius as an
expected hit, so dead panels and straws (TrackerStatus noSignal/suppress)
biased it low and made minHitRatio cuts depend on the tracker conditions.

- HelixTool takes an optional StrawNoSignal function. With it, each face
  crossing is weighted by the live fraction of the nearest straw in each
  layer at the crossing point. A function is used instead of TrackerStatus
  to avoid a Mu2eUtilities -> TrackerConditions library dependency.
- The hit ratio is evaluated lazily, on the first hitRatio() call; the
  filters apply it as the last cut, and skip it when minHitRatio <= 0.
- HelixFilter and MultiHelixFilter: new useTrackerStatus parameter
  (default true) passes TrackerStatus noSignal || suppress; false keeps
  the previous behavior. ReadTriggerInfo uses the same definition.
- Removed the unused HelixTool::nMinHitsLoop() (never set) and
  _trackerLength; nstrawhits() returns int.

With the Sim_commissioning reduced-tracker conditions (31% of straws
without signal) the median CE helix hit ratio goes from 0.56 to 1.0, so
minHitRatio 0.4 keeps its meaning. With useTrackerStatus false the
trigger results are identical to before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…strained refit

Helix finding options for trackers with fewer hits per track (e.g. the commissioning
tracker, where a CE crosses the active straws once per turn and its hits come in short
groups ~2 pi lambda apart). All default to the previous behavior.

- maxHelixCandidates (1), acceptHelixStrawHits (0): keep the best (most straw hits, then
  the lowest circle + line chi2/dof) of up to K viable candidates per search instead of
  the first; a candidate with >= acceptHelixStrawHits straw hits is saved at once.
- minAliasGap (0 = off): refit the phi-z line with the hits beyond the largest z gap of
  the helix hits shifted by -+2pi; keep the lowest chi2/dof within the momentum range,
  then recover points again.
- targetRefitSigma (0 = off), targetRefitPhiWindow (0.5 rad), targetRefitDphiDzBins (100):
  if the best of the K candidates has |d0| > targetRefitSigma, refit its circle through
  the stopping target (error targetRefitSigma) and find the phi-z line with a dphi/dz scan
  on that circle; the refit replaces the candidate if it has at least as many straw hits.
  It addresses the half-radius / half-lambda solutions that fit the hits of short arcs.

With the Sim_commissioning tracker conditions (and tuned trigger cuts), K = 3,
acceptHelixStrawHits 20, minAliasGap 100 and targetRefitSigma 100 raise the
apr_TrkDe_75m70p_D0200 efficiency for CeM from 52.9% to 57.5% (K + alias) and 61.9%
(with looser phi-z cuts and the target refit), CeP 28.5% -> 39.1%, with no increase of
the NoPrimary D0200 rate; nominal tracker +1.5%. APR finder time 0.036 -> 0.067 ms/event.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ticle

Per-event TTree comparing a helix seed collection (e.g. the trigger APR helices) with the
MC truth of the main signal particle (the highest-momentum e+- or mu+- with >= minHits
straw hits): its momentum, direction, circle and lambda at its first straw gas step, its
combo-hit z span and gaps, and for each helix its parameters, straw hits and signal straw
hits, circle chi2 with the reco and the true circle, plus the signal hits themselves.
Optionally records the decisions of trigger paths of the current process (triggerPaths),
so trigger efficiencies can be split by the signal truth (particle type, direction).
Used to study the APR helix finding with the commissioning tracker conditions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@FNALbuild

Copy link
Copy Markdown
Collaborator

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

  • CalPatRec
  • Analyses
  • Trigger
  • Mu2eUtilities
  • TrkFilters

which require these tests: build.

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

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

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at 3aa9fd8.

Test Result Details
test with ✅ Command did not list any other PRs to include
merge ✅ Merged 3aa9fd8 at fd9bfbd
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.
trigger ❓ Log file. Return Code 1.
check_cmake ✅ Log file.
FIXME, TODO ➡️ TODO (0) FIXME (1) in 7 files
clang-tidy ➡️ 6 errors 727 warnings
whitespace check ✅ no whitespace errors found

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

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

Reviewed at head 3aa9fd8776f40e409366c1aa83c6c6e752844013. first pass

Decision

  • 🟢 approve. Every new option is off by default. The trigger test confirms that the finder output is unchanged. The findings below are minor and do not block.

Scope understood

  • Three opt-in AgnosticHelixFinder options: best-of-K candidates, a 2π φ-z alias test across the largest z gap, and a refit through the stopping target. Plus a new truth analyzer, AprHelixTruthAna.
  • The branch also carries #2035 (HelixTool hit ratio). I reviewed only this PR's increment 82658397...3aa9fd87: AgnosticHelixFinder_module.cc, AprHelixTruthAna_module.cc and Analyses/CMakeLists.txt. #2035 needs its own review and should merge first.
  • No fcl changes. Nothing in Offline, Production or mu2e-trig-config turns the new options on. The only module_type : AgnosticHelixFinder table is CalPatRec/fcl/prolog.fcl:511, and TrigAprHelixFinder/TrigAprProtonHelixFinder in trig-config inherit it.

Findings

  1. 🟡 [S2] resolveGapAlias compares line fits over different hit sets when the calo cluster is on the helix.

    • Evidence: recoverPoints can recover the calo cluster. It skips only the stopping target (AgnosticHelixFinder_module.cc:1648) and puts the cluster into the line fit (:1702). resolveGapAlias then takes the current _lineFitter.chi2Dof(), which includes the cluster, as the value to beat. But it builds the ±2π alternatives without the cluster (:1732). If the alternative wins and no further hit is recovered, the saved helix has a φ-z line without the cluster, while the cluster is still used and still in the circle fit. targetRefit has the same mismatch in the other direction. Its circle refit drops the cluster (:1003), and scanDphiDz adds it back (:1853).
    • Impact: the χ²/dof comparison favours an alternative with one point fewer. The cluster's φ error is large (caloClusterSigma 30 mm), so the bias should be small. This only matters with minAliasGap > 0. I read this from the code and did not run it.
    • Suggested fix: build the baseline and both alternatives over the same points, either all with the cluster or all without it. The simplest way is to refit the baseline from idx exactly as the alternatives are built.
  2. 🟡 [S2] The six new parameters carry C++ defaults (:97-102).

    • Evidence: maxHelixCandidates 1, acceptHelixStrawHits 0, minAliasGap 0, targetRefitSigma 0, targetRefitPhiWindow 0.5, targetRefitDphiDzBins 100. The PR's own results show the right values depend on the context: the commissioning tracker gets 3/20/100/100, and offline reco may want different ones. The house rule is that such parameters stay mandatory and get their values from a prolog. The same point was raised on #2033 and #2034.
    • Suggested fix: use the two-argument fhicl::Atom form, and put the current values in the AgnosticHelixFinder table in CalPatRec/fcl/prolog.fcl. The trigger tables inherit that table through @table::, so nothing else needs to change.
  3. ⚪ [S3] A timeout throws away candidates that were already found.

    • Evidence: the timeout exit inside the triplet loop returns false (:866-870) without calling saveCandidates. With K = 1, a timeout only ever cut short a search that had found nothing. With K > 1, it can now discard a viable helix that was stored while the search went on.
    • Current impact: none. No Offline or trig-config fcl sets timeoutMs.
    • Suggested fix: save the best stored candidate on that exit when _candidates is non-empty.
  4. ⚪ [S3] The save logic is written out twice.

    • Evidence: saveAndContinue (:1061-1071) repeats the existing branch at :949-955 line for line.
    • Suggested fix: have the K = 1 branch call saveAndContinue too, so the "search for another helix" rule has one home.
  5. ⚪ [S3] Nits in AprHelixTruthAna.

    • :236 writes 11, 13, -211, -2212 where PDGCode::e_minus and its siblings should be used. Only e± and μ± can reach that line, so the -211/-2212 terms are dead.
    • The comment on _bzConv at :69 says "mm per MeV/c". The value is in MeV/c per mm, as :149 says.
    • sdmcs.at(sh) (:255, :300) assumes the straw-hit index equals the StrawDigiMC index. That holds while FilterHits is false, which it is in every Offline and trig-config table today. A one-line comment would save the next user from finding this out the hard way.

🟢 Verified at this head:

  • The defaults reproduce the old finder.
    • With maxHelixCandidates 1, the new branch is never taken (:927-928), and the old save path runs unchanged. With minAliasGap 0, resolveGapAlias is never called (:907). With targetRefitSigma 0, there is no refit.
    • FNALbuild's trigger job agrees. At this head (build 3450), all 1013 TrigReport lines are identical to #2035's own build (3449, same merge base fd9bfbdb).
    • The 8 "minor change" paths (tpr_HlxDe_70m50p +28 and so on) therefore come from #2035's HelixTool/HelixFilter change, not from this increment. #2040 at the same base matches the reference counts exactly.
  • scanDphiDz is self-consistent.
    • The slope range follows from p² = (R² + λ²)(cB)² with |dφ/dz| = 1/|λ|, clipped by maxDphiDz.
    • The circular window count uses the full width window. The adoption step keeps hits within window/2 of the window's midpoint, so the two agree.
    • The stopping-target point added in targetRefit is at (0,0), which matches _stopTargPos.
  • The analyzer's truth geometry is right. StrawGasStep positions are in the tracker frame (StrawSD.cc subtracts detectorOrigin), so d0true is measured from the tracker axis, as the helix d0 is.
  • The build is complete. The new plugin is in Analyses/CMakeLists.txt, and the scons SConscript globs it. It compiled in FNALbuild.

Validation check

  • Build/tests run:
    • FNALbuild passed at this head (build 3450, merged at fd9bfbdb). Nothing in the files this PR touches has changed on main since then.
    • trigger returned code 1, "minor change". As explained above, that comes from #2035.
    • clang-tidy reported 6 errors. The two I could see are header-environment errors (cetlib/LibraryManager.h, StopWatch.hh <format>), not this diff.
    • Locally I read the code and compared the trigger logs. I did not run anything with the new options on, so none of the opt-in paths ran in CI or here. The efficiency, background and timing tables in the PR description are the author's own numbers.
  • Config contract check: pass. The new keys are optional and no fcl sets them.
  • Cross-repo consistency: pass. Turning the options on in mu2e-trig-config is a separate change, as stated.

Residual risk

  • The opt-in paths run only when trig-config turns them on. The CI reference log will need to be regenerated in that PR.
  • Best-of-K picks by straw hits, then χ². It does not check maxEDepAvg or the direction test in saveHelix, so the chosen candidate can be dropped there while a runner-up would have passed. That was also true before, but it is now more likely.

Author follow-ups

  1. Optional: compare like with like in resolveGapAlias/targetRefit (finding 1).
  2. Optional: move the six new values into the prolog and make the atoms mandatory (finding 2).
  3. Optional: findings 3-5.
  4. Merge #2035 first.

michaelmackenzie and others added 3 commits October 19, 2026 10:01
…rackerStatus

- liveFraction: a face crossing with no straw of that face at the point (radially
  outside the panel's straws, or beyond the straw ends, e.g. between the panels of
  the face) keeps the old weight 1 instead of taking the end straws' status.
- ReadTriggerInfo: add useTrackerStatus (default true, as in HelixFilter) so the
  hit-ratio histograms can match a filter run without the tracker status.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…it options

- resolveGapAlias: refit the current line over the same points as the +-2pi
  alternatives, with a used calo cluster in all of them (as in recoverPoints).
- targetRefit/scanDphiDz: keep a used calo cluster in both the circle and the
  line fits, as in the other fits.
- A timeout during the triplet search saves the best stored candidate instead
  of dropping it.
- The K = 1 save path calls saveAndContinue, so the "search for another helix"
  rule has one home.
- The six new parameters are mandatory; their values (all options off) are set
  in the AgnosticHelixFinder table of CalPatRec/fcl/prolog.fcl.
- AprHelixTruthAna: PDGCode names, the _bzConv unit comment, and a note that the
  straw hit index is the StrawDigiMC index while FilterHits is false.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@michaelmackenzie

Copy link
Copy Markdown
Contributor Author

Thanks @oksuzian. Addressed in a336faf (with #2035's update merged in):

  1. Like-with-like fits. resolveGapAlias now refits the current line over the same points as the ±2π alternatives, with a used calo cluster in all of them (it is on the high-z side, so it is never shifted, and it is not used in the gap search). targetRefit keeps a used cluster in its circle refit, and scanDphiDz adds it to the line fit as well as the circle.
  2. The six new parameters are now mandatory. Their values, with every option off, are in the AgnosticHelixFinder table in CalPatRec/fcl/prolog.fcl, which the trig-config tables inherit.
  3. A timeout now saves the best stored candidate instead of dropping it.
  4. The K = 1 branch now calls saveAndContinue.
  5. AprHelixTruthAna: PDGCode names (the dead -211/-2212 terms are gone), the _bzConv unit comment is fixed, and a comment notes that the straw-hit index is the StrawDigiMC index while FilterHits is false.

Local check: build OK. triggerTest.fcl runs on 1000 CI events with the defaults, and also with maxHelixCandidates: 3, acceptHelixStrawHits: 20, minAliasGap: 100, targetRefitSigma: 100 on TrigAprHelixFinder (APR helix paths 36 → 38 of 1000).

@michaelmackenzie

Copy link
Copy Markdown
Contributor Author

@FNALbuild run build test

@FNALbuild

Copy link
Copy Markdown
Collaborator

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

@FNALbuild

Copy link
Copy Markdown
Collaborator

☀️ The build tests passed at a336faf.

Test Result Details
test with ✅ Command did not list any other PRs to include
merge ✅ Merged a336faf at f122c5b
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.
trigger ❓ Log file. Return Code 1.
check_cmake ✅ Log file.
FIXME, TODO ➡️ TODO (0) FIXME (1) in 7 files
clang-tidy ➡️ 6 errors 725 warnings
whitespace check ✅ no whitespace errors found

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

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

Reviewed at head a336faff6d6e1571cf1b1a1f14415ac1def0e11a. re-review of 3aa9fd87

Decision

  • 🟢 approve. All five findings from my last review are fixed. With the defaults, the finder still does exactly what it did before, and FNALbuild passes at this head apart from the trigger ❓, which comes from #2035.

Scope understood

  • Since 3aa9fd87: a merge of #2035's new commit 8e8b3d1a (reviewed under #2035), and this PR's own commit a336faff, which addresses my review. I graded only a336faff here: AgnosticHelixFinder_module.cc, CalPatRec/fcl/prolog.fcl and AprHelixTruthAna_module.cc.

Findings

  • None open.

🟢 Fixed in a336faff, verified at this head:

  • [was S2] Like-with-like fits in the alias test. resolveGapAlias now refits the current line (shift 0) over the same points as the ±2π alternatives, and takes that χ²/dof as the value to beat (AgnosticHelixFinder_module.cc:1745-1763). A used calo cluster goes into all three fits with its own 2π correction, unshifted. It sits at higher z than any tracker hit, so it belongs on the unshifted high-z side, and it is left out of the gap search. This now matches recoverPoints, which keeps the cluster in both fits (:1686, :1698).
  • [was S2] Target refit. targetRefit's circle refit now keeps a used cluster (:998). scanDphiDz still leaves the cluster out of the scan, then adds it to both the circle and the line, with a 2π correction taken from the chosen line (:1855-1864). Both fits now see the same points as the other fits.
  • [was S2] Mandatory parameters. The six new atoms have no C++ default (:97-102). Their values, with every option off, are in the AgnosticHelixFinder prolog table (CalPatRec/fcl/prolog.fcl:549-554). That table is the only module_type : AgnosticHelixFinder in Offline. Both trig-config tables inherit it with @table::. Production uses it through the AgnosticHelixFinder label, and no fcl in Offline, mu2e-trig-config, Production, PassN or EventNtuple main sets any of the six keys. fhicl-dump of the base table and of TrigAprHelixFinder and TrigAprProtonHelixFinder from trig-config main, against this prolog, gives all six keys in all three tables, with values 1, 0, 0, 0, 0.5 and 100 (claude-scratch/pr2038_review/ahf_dump.sh).
  • [was S3] Timeout. The timeout exit inside the triplet loop now saves the best stored candidate when there is one (:869-870). With K = 1 the store is always empty, so nothing changes there.
  • [was S3] One save path. The K = 1 branch now calls saveAndContinue (:949-951). The function body is the old inline code line for line, including the remaining-hits test and resetFlags.
  • [was S3] AprHelixTruthAna. It now uses PDGCode::e_minus and mu_minus, and the dead -211/-2212 terms are gone. The _bzConv comment reads "MeV/c per mm". There is a note that the straw-hit index is the StrawDigiMC index while FilterHits is false.

🟢 Still holds:

  • The defaults reproduce the old finder. With maxHelixCandidates 1 the best-of-K branch is never taken, with minAliasGap 0 resolveGapAlias is never called, and with targetRefitSigma 0 there is no refit. So none of the fit changes above runs by default. The K = 1 save path behaves as before. The only change with all options off is that the save logic now lives in one function.

Validation check

  • Build/tests run: FNALbuild passed at this head (build 3463, merged at f122c5b3). That includes ceSimReco, cosmicSimReco and check_cmake.
    • trigger is ❓ (return code 1, "minor change"). Only the 7 helix paths moved against the reference, all by less than 0.5σ. They fall about halfway between the reference and #2035's earlier build, which fits #2035's new out-of-panel rule. The table is in my #2035 review. This PR's options are off by default, so I attribute the change to #2035, but no CI run separates the two.
    • clang-tidy reports 6 errors and 725 warnings; I did not read its log.
  • Locally: I read the increment and the changed functions at this head, searched the five repositories for tables and keys, and ran the fhicl-dump above. I did not run the finder with the options on. Your check with maxHelixCandidates: 3, acceptHelixStrawHits: 20, minAliasGap: 100, targetRefitSigma: 100 (APR helix paths 36 to 38 of 1000) is your own.
  • Config contract check: pass.
  • Cross-repo consistency: pass. Turning the options on in mu2e-trig-config is a separate change.
  • Merge order: #2035 first; this branch contains it.

Residual risk

  • As in my first pass, best-of-K picks by straw hits, then χ², before saveHelix applies maxEDepAvg and the direction test. A chosen candidate can still be dropped there while a runner-up would have passed. This only matters with maxHelixCandidates > 1.
  • After a timeout, saveCandidates still runs the target refit when it is on. That is one bounded scan, but it runs after the time limit.

Author follow-ups

  • None.

@FNALbuild

Copy link
Copy Markdown
Collaborator

📝 The HEAD of main has changed to 78ea1fd. Tests are now out of date.

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