CRVResponse: fix defects found by a static sweep of the package

[oksuzian]

Nine defects found by a static review of CRVResponse at 0ef02a66d. I have not built this locally — relying on mu2e/buildtest.

⚠️ Finding 1 changes CRV simulation output. Please read it first and decide whether you want it fixed forward or handled some other way — I have no view on which, and I am happy to drop that commit and keep the rest.

---

1. 🟠 CRV sector T5 is simulated at +30.5% light yield — and has been since prolog_v11

src/CrvPhotonGenerator_module.cc:198-210 dedups MakeCrvPhotons instances by lookup-table filename. On a hit it shares the existing shared_ptr and continues, so SetScintillationYield — which only runs on the first-load branch at :223 — is never called for the second sector. The shared instance keeps the first sector's yield.

In prolog_v12.fcl, T3/T4/T5 all name CRVConditions/v6_0/LookupTable_6000_0 with scintillationYields 39794, 39794, 30482:

sector	configured yield	yield actually used
T3	39794	39794 (loads first)
T4	39794	39794 (correct by coincidence)
T5	30482	39794 ← +30.5%

It is the only mismatched filename group in the file — every other duplicate group (R1/R2/R5/L1… on 4550_0, T1/T2 on 6000_1) has identical yields. The _debug>0 line at :205 prints _scintillationYields[i], the configured value rather than the one in use, so the log actively conceals the substitution.

Scope beyond the current prolog: prolog_v11.fcl carries the identical filenames and yields. v10/v09 list only T1–T4, all at 39400, so no mismatch existed. The defect entered when T5 was added at v11 and is therefore in already-produced datasets, not only future ones.

Fixed by requiring the yields to agree as well as the filename before sharing an instance.

2. 🟠 SiPM pixel id can be one past the last pixel

src/MakeCrvSiPMCharges.cc:44-49 returned lrint of an unbounded GetRandom2 sample. The outermost [39,40) bin yields pixel id 40 in a 40×40 array — a phantom pixel with its own discharge state, inflating simulated SiPM charge. Clamped to the array.

3. 🟠 LoadLookupTable never checks stream state

src/MakeCrvPhotons.cc:275-280 reads the Cerenkov map and all three bin arrays with no check afterwards. A truncated table leaves bin boundaries and yield curves holding whatever was in the buffers and the job continues with silently wrong photon yields. Now throws.

4. 🟠 CrvWidebandTest persists indeterminate track-fit values for trigger sectors

src/CrvWidebandTest_module.cc:397-410: the continue that skips triggerSectorTypes fires before the reset block, so for every index in triggerSectorTypes ([5,6] in wideband4modules.fcl, [2,3,4,5] in widebandMuonTaggers.fcl) trackSlope/trackIntercept/trackPEs/trackPoints/trackChi2 hold whatever new[] returned on event 1 — and _tree->Fill() persists them every event, where the adjacent lines clearly intend the 0/-1 "no fit" sentinel. Moved the resets above the continue.

5. 🟡 MakeCrvDigis narrows before it saturates

src/MakeCrvDigis.cc:12-14 cast to int16_t and then applied the [minADC,maxADC] clamp, so an extreme sample wraps around and lands back inside the range instead of saturating. Clamp in double, then narrow.

6. 🟡 max_element on a possibly-empty vector

src/MakeCrvWaveforms.cc:51 — LoadSinglePEWaveform opens the file and checks good(), but a file that opens and yields no points reaches max_element on an empty vector.

7. 🟡 CrvPlot: {NAN} initializes only element 0

src/CrvPlot_module.cc:236-237 — double scale[4]={NAN}; sets element 0 to NAN and value-initializes 1–3 to 0.0. The isnan(scale[SiPM]) "no digis" guard at :305 therefore fails open for SiPM 1–3 and a flat y=0 curve is drawn; SiPM 0 conversely builds a TGaxis with a NaN bound at :287. Same bug in maxADC[4].

8. 🟡 CrvPlot: four unchecked art::Handles

:113-124 uses getByLabel and never checks isValid(). CrvMCHelper.cc:12 and CrvCoincidenceClusterMatchMC_module.cc:119 guard the same case. Now throws naming the missing product.

9. 🟡 CrvPlot's CMakeLists omits Offline::GlobalConstantsService

Despite GlobalConstantsHandle<PhysicsParams> at :102. It resolves transitively today, so this is hygiene, not a build break.

Also fixed alongside 4: CrvWidebandTest's destructor freed 3 of its 18 new[] arrays.

---

Validation

• Build: not run locally. Everything above is static — read from the source, Production and mu2e-trig-config, plus grep. Please let buildtest be the arbiter.
• No art job was run. Finding 1 is the cheapest to confirm empirically: run any CRV sim job with debug > 0 and compare the printed T5 yield against GetScintillationYield() on the instance actually used — the print is the very line that hides it.
• Lookup-table contents were not reviewed. CRVConditions/v6_0/* and data/singlePEWaveform*.txt were taken as given; finding 3 is about the absent read check, not the data.
• prolog_v03..v11 were not audited except where finding 1 required it — they are frozen history for reproducing old datasets.
• Not included, deliberately: triggerSectorTypes is a mandatory key with no default and is unset in the Production wideband config — that fix belongs in Production, not here. Two CRVTest issues (mandatory keys absent from 5 of its 6 test/ fcl; unguarded division filling NaN into two histograms) are hand-run-only and left out to keep this PR to code that production runs. Happy to open an issue with the full 16.

[FNALbuild]

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

• CRVResponse

which require these tests: build.

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

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

About FNALbuild. Code review on Mu2e/Offline.

[FNALbuild]

☀️ The build tests passed at 90abf94.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged 90abf94 at 0ef02a6
build (prof)	✅	Log file. Build time: 04 min 22 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 (2) FIXME (0) in 7 files
clang-tidy	➡️	6 errors 367 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 90abf94 after being merged into the base branch at 0ef02a6.

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.