-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Working Version PixelCPECluster Repair (still turned off) #25727
Conversation
…flag bug in CPEBase, updated 2d template fit for stability, and added tunable qratio and nydiff parameters for clusterRepair
…uster repair to run 1d or 2d but not both
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25727/8121
|
A new Pull Request was created by @OzAmram (Oz Amram) for master. It involves the following packages: RecoLocalTracker/SiPixelRecHits @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25727/8153
|
Based on the Pixel offline meeting this morning. It was decided to keep Cluster Repair turned off in this PR. If after discussion and validation with other groups, it is determined that we want these changes in for the UL re-reco then we will submit another PR turning on CR. I am editing the title accordingly. |
please test |
The tests are being triggered in jenkins. |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25727/8201
|
Just pushed a new commit that covers the comments in the code review. In SiPixelTemateReco2D I switched all of the fabs to std::abs and removed the un-needed fabs that was pointed out. |
please test |
The tests are being triggered in jenkins. |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
I've tried to run it by enabling the CPE cluster repair, but of course I miss the needed template. |
Please use this GT for any data run: 104X_dataRun2_pixel2DTemplate_Cand_v1 |
Comparison is ready Comparison Summary:
|
+1
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
This PR is meant to fully implement the new pixel CPE Cluster Repair.
We are making the changes discussed in the Dec. 14th reco meeting (slides here).
These changes include:
Our homework from that meeting was to measure the cost of these changes in CPU time and the benefits in terms of higher level observables. We have done both and attached some slides showing our results. We will be presenting these slides at the Pixel Offline Meeting on Wed. Jan 23rd.
Slides here.
We are still working on generating all the needed 2D templates for different data taking IOV's so we current expect that many of the automated tests will fail unless they run on a specific IOV's that we currently have templates for and use specific Global Tags. More IOV's will be continually added.
EDIT:
This PR still leaves CPE Cluster Repair turned off. To turn it on you only need to uncomment the two lines here
@perrotta @slava77 @fabiocos @makortel @tvami @tsusa @OzAmram @cmantill @schuetzepaul @pmaksim1