Closed sebouh137 closed 3 months ago
github-actions[bot]
commented
3 months ago 
sebouh137
commented
3 months ago Dear @wdconinc ,
All checks except for the eic_container check, which appears to be unrelated to my pull request, have succeeded and all conversations have been resolved. If you don't have any further comments or suggested changes, please approve this pull request.
Best regards,
Sebouh
wdconinc
commented
3 months ago I am still wondering why the number of clusters in the hcal insert goes down by a factor 4 or so. Do you expect this or is the new x,y cluster much less efficient?
sebouh137
commented
3 months ago I am still wondering why the number of clusters in the hcal insert goes down by a factor 4 or so. Do you expect this or is the new x,y cluster much less efficient?
I believe there are two things that have changed. First, groups of hits that, without the tweaks to these values, would not have connected to form a cluster are merged into a single cluster. Second, I changed the minimum number of hits per cluster as well as the thresholds in the topo clustering config, since we're using subcell hits from HEXPLIT instead of regular hits at the cell level. Requiring 10 subcell hits doesn't really do anything, since an isolated cell-level hit would be split into 12 subcell hits in HEXPLIT and therefore would easily be its own cluster.
So, what we're seeing makes sense to me.
sebouh137
commented
3 months ago Dear @wdconinc,
All conversations have been resolved. Could you approve this pull request?
Thanks,
Sebouh
wdconinc
commented
1 month ago This algorithm would not have been merged if janatop had worked at the time this was reviewed :-\
Top 10 factories by cpu consumption:
Factories:
2.02 s ( 0.4%) edm4eic::CalorimeterHit:HcalFarForwardZDCRecHits
2.75 s ( 0.5%) Acts::TrackContainer<Acts::ConstVectorTrackContainer, Acts::ConstVectorMultiTrajectory, std::shared_ptr>:CentralCKFActsTracksUnfiltered
2.89 s ( 0.5%) edm4eic::ProtoCluster:LFHCALIslandProtoClusters
3.27 s ( 0.6%) edm4eic::Measurement2D:CentralTrackerMeasurements
5.81 s ( 1.1%) edm4eic::CalorimeterHit:HcalFarForwardZDCSubcellHits
5.86 s ( 1.1%) edm4eic::TrackSegment:CalorimeterTrackProjections
10.29 s ( 2.0%) edm4eic::ProtoCluster:HcalEndcapPInsertImagingProtoClusters
11.69 s ( 2.2%) edm4eic::ProtoCluster:HcalFarForwardZDCIslandProtoClustersBaseline
29.41 s ( 5.6%) edm4eic::CherenkovParticleID:DRICHAerogelIrtCherenkovParticleID
434.49 s ( 82.5%) edm4eic::ProtoCluster:HcalFarForwardZDCImagingProtoClusters
Briefly, what does this PR introduce?
Use Δx and Δy for determining which hits in adjacent layers are neighbors to one another in both the forward hadronic calorimeter insert and the ZDC.
What kind of change does this PR introduce?
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
no
Does this PR change default behavior?
yes. Uses new Δx, Δy mode for the calorimeter insert and the ZDC