search

cms-L1TK / cmssw

Fork of CMSSW where improvements to L1 tracking code are developed.
http://cms-sw.github.io/
Apache License 2.0
4 stars 5 forks source link

Move module loop to setup #175

Closed Jingyan95 closed 2 years ago

Jingyan95 commented 2 years ago

Moving expensive (module) loops to Setup.cc which is called at BeginRun.

tomalin commented 2 years ago

Please say in Description if you've checked this reduces CPU use.

tomalin commented 2 years ago

I believe the HPH is used only in L1TrackNtupleMaker.cc to calculate variables such as tmp_trk_nPSstub_hitpattern that are added to the TTree? So if HPH contains bugs, they won't show up (except if compilation errors) in the CI test checks of tracking efficiency? Have you therefore checked that these variables look sensible for both NewKF & OldKF?

tomalin commented 2 years ago

The Hybrid code crashed in the CI test https://gitlab.cern.ch/cms-l1tk/cmssw_CI/-/pipelines/4155053 .

Jingyan95 commented 2 years ago

@tomalin I moved the HPH files under TrackFindingTracklet package such that Thomas's functions from TrackerTFP can be included without creating circular dependency.

Jingyan95 commented 2 years ago

Hi @tomalin , I wanted to check if you had some time to look at this PR ? The primary comment (CPU usage) was addressed by the recent commit and the hitpatternhelper module now uses very minimal CPU.

tomalin commented 2 years ago

@tomalin I moved the HPH files under TrackFindingTracklet package such that Thomas's functions from TrackerTFP can be included without creating circular dependency.

@Jingyan95 Can you please explicitely answer each of the review comments. Whilst some remain unanswered, I assume the work's not yet been done, so don't bother rereviewing.

Jingyan95 commented 2 years ago

Please say in Description if you've checked this reduces CPU use.

CPU usage is negligible now

Jingyan95 commented 2 years ago

I believe the HPH is used only in L1TrackNtupleMaker.cc to calculate variables such as tmp_trk_nPSstub_hitpattern that are added to the TTree? So if HPH contains bugs, they won't show up (except if compilation errors) in the CI test checks of tracking efficiency? Have you therefore checked that these variables look sensible for both NewKF & OldKF?

The performance of the HPH has been checked and it is consistent with the previous version Old KF: TTbar_PU200_D76_FPR_hitpattern.pdf TTbar_PU200_D76_TPR_hitpattern.pdf New KF: TTbar_PU200_D76_NEWKF_FPR_hitpattern.pdf TTbar_PU200_D76_NEWKF_TPR_hitpattern.pdf

skinnari commented 2 years ago

hi @tomalin @Jingyan95 it seems that all comments have been addressed, can this PR be merged? thanks!