Skip to content

Conversation

@edcallaghan
Copy link
Contributor

This PR implements digi mixing for the calorimeter. The mechanics largely follow the pattern set by the tracker: the relevant data products are mixed using standard technology, and digis from the same channel which overlap in time are resolved into a single digi. For calorimeter digis, which are principally waveforms, this is conceptually straightforward: the waveforms are summed. It will, however, require extra effort to retain MC info for the calorimeter: unlike the tracker or CRV, which instantiate all MC info at the time of digi creation, it seems that some MC info for the calorimeter is reconstructed, making use of primary SimParticle info, after digis have already been made and interpreted as hits. That should be fine, but it require extra book-keeping/massaging to work in situations where there is MC info (i.e. detector data) or some digis were digitized independently of the simulated signal (e.g. digi mixing with conventional pileup).

New classes:

  • CaloDigiWrapper: A thin wrapper for calorimeter digis which is meant to collapse multiple loops, i.e. over synchronized digi + mc info collections, into a single loop. MC info is yet not included, for reasons described above.
  • CaloDigiWrapperCollection: stl-vector of the above, with additional logic for resolving a set of overlapping digis into one.

Changes to existing functionality:

  • Mu2eProductMixer: Include provisions for mixing CaloDigis and CaloShowerSims, which (it seems to me) is the principal data product, not already mixable, which will allow to retain MC info.
  • TimeBasedBuckets: Generalized from a sequence of fixed-length timing windows to a sequence of variable-length timing windows. This is necessary as waveforms from the calorimeter of of variable length.

Changes to existing modules:

  • MergeDigis: Resolve collisions between overlapping calorimeter digis using CaloDigiWrapperCollection functionality.

Thoughts looking forward: The simple sum of multiple waveforms into a single waveform admits two subtleties: the saturation of the ADC must be reevaluated, and any analog noise present in the waveform is now "counted" multiple times in the sum.
The former is accounted for in this PR, albeit with the ADC bit depth configuration now duplicate, as it is supplied directly to the CaloDigiMaker module, and not factored into some kind of conditions class. It would be good for that to be done, or maybe move the definition to fcl-prolog, to avoid the possibility of them getting out of sync.
The latter is not accounted for, as currently an implemented waveform is just that: a waveform; it does not know "what" it contains. Conceptually, one can imagine ways of keeping the noise contribution to waveforms distinct until it absolutely needs to be added to any signal present, but that is outside of the scope of this PR and I'm not in a position to recommend something right now. But it's something we'll want to get right, since potential applications include mixing digis from the detector with both detector data and MC, both in situations with overlapping digis and not, so artefacts associated with different levels of noise are possible.

@FNALbuild
Copy link
Collaborator

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

  • CaloMC
  • TrackerMC
  • EventMixing
  • Mu2eKinKal
  • Blinding

which require these tests: build.

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

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

About FNALbuild. Code review on Mu2e/Offline.

@FNALbuild
Copy link
Collaborator

☔ The build is failing at b3cee8b.

scons: *** [build/al9-prof-e29-p090/Offline/tmp/Blinding/src/MergeDigis_module.os] Error 1
scons: *** [build/al9-prof-e29-p090/Offline/tmp/EventMixing/src/MixDigis_module.os] Error 1
scons: *** [build/al9-prof-e29-p090/Offline/tmp/EventMixing/src/MixBackgroundFrames_module.os] Error 1
scons: *** [build/al9-prof-e29-p090/Offline/tmp/EventMixing/src/Mu2eProductMixer.os] Error 1
scons: *** [build/al9-prof-e29-p090/Offline/tmp/EventMixing/src/ResamplingMixer_module.os] Error 1
Test Result Details
test with Command did not list any other PRs to include
merge Merged b3cee8b at 5d5efb9
build (prof) Log file.
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 (17) FIXME (5) in 10 files
clang-tidy 🔶 24 errors 24 warnings
whitespace check no whitespace errors found

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

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.

@edcallaghan
Copy link
Contributor Author

@FNALbuild run build test

@FNALbuild
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

☔ The build is failing at ae1489f.

scons: *** [build/al9-prof-e29-p090/Offline/tmp/Blinding/src/MergeDigis_module.os] Error 1
scons: *** [build/al9-prof-e29-p090/Offline/tmp/EventMixing/src/MixDigis_module.os] Error 1
scons: *** [build/al9-prof-e29-p090/Offline/tmp/EventMixing/src/MixBackgroundFrames_module.os] Error 1
scons: *** [build/al9-prof-e29-p090/Offline/tmp/EventMixing/src/Mu2eProductMixer.os] Error 1
scons: *** [build/al9-prof-e29-p090/Offline/tmp/EventMixing/src/ResamplingMixer_module.os] Error 1
Test Result Details
test with Command did not list any other PRs to include
merge Merged ae1489f at 5d5efb9
build (prof) Log file.
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 (17) FIXME (5) in 10 files
clang-tidy 🔶 19 errors 34 warnings
whitespace check no whitespace errors found

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

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.

@edcallaghan
Copy link
Contributor Author

@FNALbuild run build test

@FNALbuild
Copy link
Collaborator

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

@edcallaghan
Copy link
Contributor Author

@FNALbuild run build test

@FNALbuild
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

☀️ The build tests passed at 479180c.

Test Result Details
test with Command did not list any other PRs to include
merge Merged 479180c at 5d5efb9
build (prof) Log file. Build time: 04 min 31 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. Return Code 1.
FIXME, TODO 🔶 TODO (17) FIXME (5) in 10 files
clang-tidy 🔶 4 errors 39 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 479180c after being merged into the base branch at 5d5efb9.

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.

@edcallaghan
Copy link
Contributor Author

edcallaghan commented Dec 23, 2025

@bechenard I've requested your review based on authorship of existing code, but obviously please delegate/defer as appropriate. The real calorimeter action is in CaloDigiWrapperCollection::ResolveCollision and ResolveCollisions. The latter may look a bit opaque, but is just identifying which digis have waveforms which overlap in time. The former performs the waveform sum.

Also note that f56bda5 patches what I think is a (very minor) bug in the digitization which allows the simulation to produce adc readings which are one count above the true max reading, but I'll reverse that if I misunderstood.

@rlcee
Copy link
Collaborator

rlcee commented Dec 23, 2025

In the CI report, check_cmake is reporting a problem, please take a look at that. Thanks

@edcallaghan
Copy link
Contributor Author

@FNALbuild run build test

@FNALbuild
Copy link
Collaborator

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

@FNALbuild
Copy link
Collaborator

☀️ The build tests passed at 71a8ca4.

Test Result Details
test with Command did not list any other PRs to include
merge Merged 71a8ca4 at 5d5efb9
build (prof) Log file. Build time: 04 min 31 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 (16) FIXME (5) in 11 files
clang-tidy 🔶 6 errors 73 warnings
whitespace check no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 71a8ca4 after being merged into the base branch at 5d5efb9.

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.

@edcallaghan
Copy link
Contributor Author

@rlcee thanks, I forgot to propagate new classes into CMakeLists.

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.

3 participants