[michaelmackenzie]

This is useful for identifying high energy proton helices online. If cutting on the mass is configured, unphysical (negative) masses are filtered out.

[FNALbuild]

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

• 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 df23eca: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

[FNALbuild]

☀️ The build tests passed at df23eca.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged df23eca at 51a5125
build (prof)	✅	Log file. Build time: 04 min 27 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.
check_cmake	✅	Log file.
FIXME, TODO	➡️	TODO (0) FIXME (1) in 1 files
clang-tidy	➡️	1 errors 28 warnings
whitespace check	✅	no whitespace errors found

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

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]

PR Review Summary — #2057

Reviewed at head df23eca2ef357359f41c9bda17db1641f4921b24. First pass.

Decision

• 🟢 approve

Scope understood

• HelixFilter gains two optional cuts, minMass and maxMass, on a mass estimated from the helix momentum and the time-vs-z slope of the helix hits.
• If either key is set, a helix whose estimate is unphysical (m² ≤ 0) fails. If neither is set, the cut is skipped.
• No table in mu2e-trig-config main (2241f345) sets either key yet.

Findings

1. 🟡 [S2] A maxMass-only cut also rejects electrons whose t-vs-z slope reads low, so it does not work as a proton veto
   • Evidence: the condition at HelixFilter_module.cc:172 requires mass > 0.f whenever _useMass is true, and _useMass is true when only maxMass is set (:97). For an electron, β is 1 to better than 10⁻⁵, so the true m² is about 0. The measured m² is ≈ 2δp², where δ is the relative error on the slope. Its sign is the sign of δ. With the formula at :146-147, an electron with p = 105 MeV and cos θ = 0.7 gives 0.5 MeV at the true slope. With the slope 1% low it gives m² < 0, so mass = -1 and the helix fails. With the slope 1% high it gives 14.9 MeV. A 300 MeV proton stays between 886 and 990 MeV for slope errors from -5% to +5%. Script: claude-scratch/offline_pr2057_demo/mass_demo.sh, available on request.
   • Impact: none today, because nothing sets the keys. The stated use, selecting protons with minMass, works as intended. But a config that keeps light particles with maxMass alone, as a veto, drops electrons whose slope reads low. If the slope estimate is unbiased, that is about half of them. I did not measure the slope bias or resolution on real helices.
   • Suggested fix: require m2 > 0 only when minMass is set. Alternatively, cut on signed m² (m2 <= maxMass²), so that β > 1 passes a maximum. Either way, keep failing a helix with no slope measurement.

Verified

• 🟢 The slope is dt/dz in ns/mm, as the new comment says. HelixTool::dirOfProp (Mu2eUtilities/src/HelixTool.cc:104-118) fits correctedTime() against pos().z() with LsqSums2, x = z and y = t, and returns dydx(). AgnosticHelixFinder (:1660-1662), CalHelixFinder (:457-460) and RobustHelixFinder (:561-565) store it in _recoDir.
• 🟢 The formula and units are right. E = p/β = p_z/β_z, and 1/β_z = c·|dt/dz| with CLHEP::c_light = 299.79 mm/ns. At the true slope the script returns 0.5 MeV for the electron and 938.3 MeV for the proton. fabs makes upstream-going helices work.
• 🟢 Degenerate slopes fail. LsqSums2::dydx() returns 0 with fewer than 2 points (LsqSums2.cc:80-88), so m² = -p² and the mass is -1. Hits all at one z give inf or NaN. NaN fails m2 > 0.f, and inf fails mass <= _maxmass because _maxmass is DBL_MAX.
• 🟢 _useMass = config.minMass(_minmass) | config.maxMass(_maxmass) (:97) uses a bitwise | on purpose. Both OptionalAtom reads run. A || would skip reading maxMass whenever minMass is set.
• 🟢 The cut is a true no-op when unset. _useMass, _minmass and _maxmass are initialized at declaration (:209-211), and the condition opens with !_useMass ||. FNALbuild build 3488 at this head is green, and the trigger test returns 0 ("No problems detected").

Validation check

• Build/tests run: no Offline build or art job. I ran a standalone evaluation of the new formula (finding 1). FNALbuild 3488 passed every test. The one new clang-tidy note is a double-to-float narrowing at :146, which is harmless here.
• Config contract check: pass. Two new optional keys; existing tables validate unchanged.
• Cross-repo consistency: no companion change is needed to merge.

Residual risk

• MPR helices carry no slope. RobustMultiHelixFinder never sets _recoDir (TrkPatRec/src/RobustMultiHelixFinder_module.cc:355-367), so its slope is 0 and every one of its helices fails a mass cut. The one HelixFilter on MPR helices is MprTrkDe80m70pD0200HSFilter (trigMprFilters.fcl:7-9). Its slope-significance cuts have the same limitation today.
• The mass inherits HelixFilter's momentum scale. mm2MeV = 3./10. assumes Bz = 1 T, and m = p·√(1/β² − 1) scales linearly with p. At the nominal field this is exact. At any other field, the mass is off by the same factor as the momentum cuts.
• Merge order with #2035: git merge-tree of this head with #2035's head 115db196 reports a content conflict on the debug print at :153-154, the same statement as before. Whichever merges second needs a one-line rebase.

Author follow-ups

1. Is maxMass meant to be usable as a veto? If so, see finding 1.
2. MultiHelixFilter does not get the cut, like the eDepAvg cut in #2055. Is that intended?

[oksuzian]

PR Review Summary — #2057

Reviewed at head b463f5a369957eb5ae8830fca6cdf555d5ea1942. re-review of df23eca

Decision

• 🟢 approvable. There is no build at this head yet, and main has moved since the last one (#2058 and #2051 merged), so I am posting this as a comment and triggering a build. I will approve when it is green.

Scope understood

• One new commit, b463f5a3. The single _useMass flag becomes _useMinMass and _useMaxMass. A helix must have a real mass only when minMass is set. maxMass now compares the computed value directly.

Carried forward from the review at df23eca2

1. 🟢 [was S2] A maxMass-only cut rejecting electrons whose slope reads low: FIXED. With only maxMass set, the condition is mass <= _maxmass (HelixFilter_module.cc:174). An unphysical estimate gives mass = -1 (:148), which now passes. So an electron whose t-vs-z slope reads 1% low passes, as one reading 1% high does (14.9 MeV). The minMass branch keeps mass > 0.f (:173), so proton selection behaves as before.

Findings

1. ⚪ [S3] With only maxMass set, a helix with no slope measurement now passes
   • Evidence: a slope of 0 gives m2 = -p², so mass = -1, which passes mass <= _maxmass. That covers helices with fewer than 2 usable hits, and every helix from RobustMultiHelixFinder, which never sets _recoDir. A NaN slope also gives mass = -1 and passes.
   • Impact: none today, because no table sets either key. For a proton veto, "no measurement, do not veto" is a reasonable choice. My first review suggested the opposite, so this is a note, not a request.
   • Suggested fix: optional. Say it in the maxMass comment: "helices without a usable t-vs-z slope pass".

Verified

• 🟢 Both keys are still read independently, at :97-98, and both flags are initialized at declaration (:211-212). With neither key set, the two new conditions are true, so the filter is unchanged.
• 🟢 Nothing else changed. The compare from df23eca2 to this head is this one commit, which touches only HelixFilter_module.cc (+6 -3).

Validation check

• Build/tests run: no local build. I re-read the changed lines at this head. FNALbuild has not built this head; I am triggering it with this review.
• Config contract check: pass. The keys are unchanged.
• Cross-repo consistency: pass. No trig-config table sets the keys yet.

Residual risk

• Unchanged from the first review: the mass uses HelixFilter's 3./10. momentum conversion, and this PR still conflicts with #2035 on the debug print.

Author follow-ups

1. Optional: document the no-slope behaviour of maxMass (finding 1).

[oksuzian]

@FNALbuild run build test

[FNALbuild]

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

[FNALbuild]

☀️ The build tests passed at b463f5a.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged b463f5a at 4c00368
build (prof)	✅	Log file. Build time: 04 min 27 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.
check_cmake	✅	Log file.
FIXME, TODO	➡️	TODO (0) FIXME (1) in 1 files
clang-tidy	➡️	1 errors 27 warnings
whitespace check	✅	no whitespace errors found

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

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.