increment iDigi in out-of-time condition

[EBerzin]

I am updating ldmx-sw, here are the details.

What are the issues that this addresses?

This resolves #1592.

Check List

• I successfully compiled ldmx-sw with my developments
• I ran my developments and the following shows that they are successful.

Here is some log output showing the change relative to the behavior documented in the issue.

[ Process ] 44  info: Processing 44 Run 10036 Event 44  (2025-02-11 11:12:06.304602000-0800)
[ trigScintClustersPad1 ] 44 debug: TrigScintClusterProducer: produce() starts! Event number: 44
[ trigScintClustersPad1 ] 44 debug: Got digi collection trigScintRecHitsPad1_overlay with 2 entries 
[ trigScintClustersPad1 ] 44 debug: Mapping digi hit nb 1 with energy = 0.291239 MeV, nPE = 72.8098 > 0 to key/channel 29
[ trigScintClustersPad1 ] 44 debug: 	 At hit with channel nb 29.
[ trigScintClustersPad1 ] 44 debug: Seeding cluster with channel 29; content 72.8098
[ trigScintClustersPad1 ] 44 debug:    In addHit, adding hit at 29 with amplitude 72.8098, updating cluster to current centroid 29 and energy 72.8098. index vector now ends with 29
[ trigScintClustersPad1 ] 44 debug: 	 itr is pointing at hit with channel nb 29.
[ trigScintClustersPad1 ] 44 debug: Now have 1 hits in the cluster 
TrigScintCluster { Energy: 0.291239, Number of hits: 1, Seed channel 29, Channel centroid: 29 }
  --  Constituent hit channel ids: {  29  }
[ trigScintClustersPad1 ] 44 debug: 	 Finished processing of seeding hit with channel nb 29.
[ trigScintClustersPad2 ] 44 debug: TrigScintClusterProducer: produce() starts! Event number: 44
[ trigScintClustersPad2 ] 44 debug: Got digi collection trigScintRecHitsPad2_overlay with 1 entries 
[ trigScintClustersPad2 ] 44 debug: Mapping digi hit nb 0 with energy = 0.490913 MeV, nPE = 122.728 > 0 to key/channel 29
[ trigScintClustersPad2 ] 44 debug: 	 At hit with channel nb 29.
[ trigScintClustersPad2 ] 44 debug: Seeding cluster with channel 29; content 122.728
[ trigScintClustersPad2 ] 44 debug:    In addHit, adding hit at 29 with amplitude 122.728, updating cluster to current centroid 29 and energy 122.728. index vector now ends with 29
[ trigScintClustersPad2 ] 44 debug: 	 itr is pointing at hit with channel nb 29.
[ trigScintClustersPad2 ] 44 debug: Now have 1 hits in the cluster 

Here the high-PE hit is correctly mapped to index 1, and we successfully form a cluster in Pad 1.
The effects of this change on out-of-time pileup samples are also documented here.

[EBerzin]

The plots that fail the PR Validation are all cluster-related, as we would expect.

They show a decreased cluster time and increased beamEfrac (due to the removal of out-of-time hits or noise hits), and a decreased number of single-hit clusters.

TrigScintClusterPad3_TrigScintClusterPad3_cluster_time.pdf
TrigScintClusterPad3_TrigScintClusterPad3_n_hits.pdf
TrigScintTracks_TrigScintTracks_beamEfrac.pdf

[bryngemark]

I no longer remember the logic of why iDigi was incremented the way it was. The way you do this here, it looks like we shouldn't think of it as a way to keep track of added hits (increment iDigi) but instead we should use the hit index in the hits vector directly in the map.

Have you checked that this works out well also without out-of-time PU (you have to run a lot of sim to get late hits, or, you could increase the timespread/timemean to get a good population from overlay)? Trying to convince myself that this solution is the way to go – I am fully aware of the performance improvement it brings, just want to make sure we're not introducing new bugs.

[EBerzin]

I think the way the rest of clustering is implemented, iDigi should be just the index of the hit in the hit collection (not the index of added hits). The values in hitChannelMap_ are used to directly access the digi collection, as in ldmx::TrigScintHit digi = (ldmx::TrigScintHit)digis.at(itr->second); (where itr is over hitChannelMap_). So I think we want to increment iDigi for all digis when forming hitChannelMap_, regardless of if they are added or not.

I have run this on samples without out-of-time pileup, and it doesn't seem to cause problems (we actually see a slight efficiency improvement).

[bryngemark]

ok thanks for verifying @EBerzin, i simply don't remember what i did 5 years ago all that well :D

i think the efficiency improvement is expected across the board.