Include calorimeter in digi harvesting

[edcallaghan]

This PR includes calorimeter digis in the workflow of extracting digis from an event which participate in a reconstructed track and doctoring their timing to change where in the event window they (and, as a consequence, a track reconstructed them) occur in the event window, which is necessary to include the calorimeter in the proposed salt-with-cosmics blinding scheme.

Changes to existing modules:

• TrackDigiExtractor: Identify all calorimeter digis which ultimately contribute to a calorimeter hit associated with a given fit track, and store them in a dedicated data product, similar to how tracker digis are handled.
• DisplaceDigiTimes: Deep-copy of all calorimeter digis in a given collection, with the starting timestamp shifted by a predetermined interval (subject to the sample spacing of the waveforms, which are the timestamp units).

One note is that the sample spacing of the calorimeter is not stored condition available via the usual handes, but (where used elsewhere) as a fcl parameter; this implementation thus also supplies this value as a fcl parameter, which may be coordinated as a prolog value when production fcl for this workflow materializes.

This also sneaks in some extra declarations of data products to be serialized to disk, which are necessary when running certain ensembling workflows in the process of testing the blinding mechanics.

[FNALbuild]

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

• Blinding
• MCDataProducts

which require these tests: build.

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

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

About FNALbuild. Code review on Mu2e/Offline.

[FNALbuild]

☀️ The build tests passed at a4adb5e.

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

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

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 #1814 Review: Include calorimeter in digi harvesting

1) Summary

Author: @edcallaghan • State: open, mergeable (clean) • Risk: Low–Medium
Scope: 3 files, +80 / −4, 2 commits, 0 reviews, 2 bot comments
Label: build finished ✅ — All FNALbuild build/integration tests pass at a4adb5e. Note: clang-tidy reports 4 errors, 24 warnings (log).

The PR extends the blinding/digi-harvesting workflow to include the calorimeter — previously // calorimeter... placeholder stubs in two modules. It also registers some art::Sampled<...> dictionary entries needed when running ensembling jobs that exercise the blinding chain.

2) Core changes

A. TrackDigiExtractor_module.cc — extract CaloDigis associated with a track

Previously only tracker digis were collected. Now, for each KalSeed with a calo cluster, walks caloCluster → caloHits → recoCaloDigis → caloDigiPtr and deep-copies each CaloDigi into an output CaloDigiCollection.

if (kalseed.hasCaloCluster()){
  const auto& cseed = kalseed.caloHit();
  const auto& cluster = cseed.caloCluster();
  const auto& hits = cluster->caloHitsPtrVector();
  for (const auto& hit: hits){
    const auto& recos = hit->recoCaloDigis();
    for (const auto& reco: recos){
      const auto& ptr = reco->caloDigiPtr();
      auto digi = CaloDigi(ptr->SiPMID(), ptr->t0(),
                           ptr->waveform(), ptr->peakpos());
      ext_calodigis->push_back(digi);
    }
  }
}

B. DisplaceDigiTimes_module.cc — shift calo digi start times

Adds an additional fcl input CaloDigiCollection and a new fcl atom CaloADCSamplePeriod. For each input calo digi, the existing shift (computed for the tracker case) is added to the analog start time t0 * sample_period, then converted back to integer sample units.

auto analog = t0 * sample_period;
auto shifted = analog + shift;
t0 = static_cast<CaloDigiWrapper::sample_t>(shifted / sample_period);
cdigis->emplace_back(id, t0, waveform, peakpos);

C. MCDataProducts/src/classes_def.xml — sampled product dictionaries

Adds art::Sampled<...> + art::Wrapper<...> + the underlying std::map<art::SubRunID,...> entries for ProtonBunchIntensity and GenEventCount, needed for sampling/ensembling jobs.

---

3) Issues found

🔴 Bug: typo mu2e: (single colon) in classes_def.xml

Six lines use mu2e: instead of mu2e::. The same lines that include the underlying std::map<...,mu2e::ProtonBunchIntensity> use the correct double colon, which strongly suggests this is a typo. ROOT/genreflex will likely fail to resolve these as the intended type.

<class name="art::Sampled<mu2e:ProtonBunchIntensity>"/>                              <!-- mu2e: -->
<class name="art::Wrapper< art::Sampled<mu2e:ProtonBunchIntensity> >"/>              <!-- mu2e: -->
<class name="std::map<art::SubRunID,mu2e::ProtonBunchIntensity>"/>                   <!-- mu2e:: ✓ -->
<class name="art::Wrapper< std::map<art::SubRunID,mu2e::ProtonBunchIntensity> >"/>   <!-- mu2e:: ✓ -->
<class name="std::map< std::string,std::map<art::SubRunID,mu2e::ProtonBunchIntensity> >"/>

<class name="art::Sampled<mu2e:GenEventCount>"/>                                     <!-- mu2e: -->
<class name="art::Wrapper< art::Sampled<mu2e:GenEventCount> >"/>                     <!-- mu2e: -->

Fix: change all mu2e: → mu2e:: in the new Sampled/Wrapper<Sampled> lines.

🟡 Truncation (not rounding) when converting shifted time back to sample index

static_cast<sample_t>(shifted / sample_period) truncates toward zero. For a non-integer multiple of sample_period, the calo digi will land on the lower sample bin while the tracker shift uses the unmodified continuous value. Two consequences:

• The actual time shift applied to calo waveforms differs from the one used for the track/straw digis by up to one sample period (~ tens of ns for the calo).
• Behavior is asymmetric for negative shifted values (truncation toward zero vs. floor).

Consider static_cast<sample_t>(std::lround(shifted / sample_period)) (or std::floor) and document the chosen convention. If this discrepancy is intentional (because the calo is inherently sample-quantized), a comment justifying it would help.

🟡 No bounds/validity check on kalseed.caloHit()

hasCaloCluster() is gated, but if caloHit().caloCluster() is ever a null art::Ptr, the cluster->caloHitsPtrVector() deref will segfault. Worth either a defensive if (cluster.isNonnull()) check or a comment that the upstream contract guarantees this.

🟡 Potential duplicate CaloDigis across KalSeeds

If two KalSeeds share a CaloCluster (or share constituent CaloHits/RecoCaloDigis), the same underlying CaloDigi will be deep-copied multiple times into ext_calodigis. The tracker side has the same theoretical exposure, but for calo the cluster-sharing pattern is more common. Consider deduping by art::Ptr<CaloDigi> (e.g., std::set of pointers) before emplacing — or document that downstream consumers tolerate duplicates.

🟢 Minor: identical fhicl Comment for new and existing inputs

Both StrawDigiCollection and the new CaloDigiCollection atoms in DisplaceDigiTimes use the comment "art::InputTag of digis to displace". Distinguishing them ("…of straw digis…" / "…of calorimeter digis…") would help fcl readers.

fhicl::Atom<art::InputTag> calo_digi_tag{
  fhicl::Name("CaloDigiCollection"),
  fhicl::Comment("art::InputTag of digis to displace")   // copy of StrawDigiCollection comment
};

🟢 Minor: peakpos unmodified after time shift

The shifted digi keeps the original peakpos. Since peakpos is typically expressed in samples relative to t0, this is probably correct, but worth confirming — and worth a one-line comment in the loop, since reviewers will ask.

🟢 Minor: CaloADCSamplePeriod as fcl param

Acknowledged in the PR description. Track this as a TODO (conditions-based sampling period) and ensure the prolog value is consistent with what CaloDigiMaker/CaloHitMaker use elsewhere, otherwise the round-trip will be inconsistent.

---

4) Merge readiness

Aspect	Status
Build & integration tests	✅ all green
Mergeable	✅ clean
Reviews	⏳ none yet
clang-tidy	⚠️ 4 errors, 24 warnings — worth a glance to confirm none are introduced by this PR

Blocker for merge, in my view: the mu2e: vs mu2e:: typo in classes_def.xml. The other items are improvements / clarifications.

---

5) Suggested improvements

1. Fix the mu2e: → mu2e:: typos.
2. Use std::lround instead of truncation for the calo t0 conversion, or document the truncation choice.
3. Differentiate fcl Comment strings for the two InputTag atoms in DisplaceDigiTimes.
4. Consider deduplicating CaloDigis across KalSeeds in TrackDigiExtractor.
5. Add a brief comment near peakpos in the displaced digi to clarify it is intentionally not shifted.

---

6) Want me to…

1. Inspect the clang-tidy log to confirm whether any of the 4 errors / 24 warnings are introduced by this PR.
2. Pull CaloDigi.hh / CaloDigiWrapper.hh to confirm the sample_t type and recommend lround vs floor precisely.
3. Check KalSeed::caloHit() / hasCaloCluster() semantics in the codebase for the null-pointer concern.
4. Search for existing fcl files referencing DisplaceDigiTimes / TrackDigiExtractor to verify they will need to be updated for the new required parameters (these are required atoms with no defaults — existing fcl will break until updated).
5. Draft a suggested patch comment for the classes_def.xml typo fix.

[FNALbuild]

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