make CPEFast to better reproduce Generic (w/o track angle) #34286
Conversation
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34286/23600
|
|
A new Pull Request was created by @czangela for master. It involves the following packages: CUDADataFormats/TrackingRecHit @perrotta, @makortel, @slava77, @cmsbuild, @fwyzard, @jpata can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild please test |
|
enable gpu |
cmsbuild
added
code-checks-pending
tests-pending
and removed
code-checks-approved
tests-started
labels
|
@cmsbuild, please test |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34286/23606
|
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cb902c/16347/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: GPU Comparison SummarySummary:
Comparison SummarySummary:
|
|
+heterogeneous |
|
Expecting a follow up PR that adds back the |
CUDADataFormats/TrackingRecHit/interface/TrackingRecHit2DHeterogeneous.h
Show resolved
Hide resolved
| //float pt = std::min<float>(tracks->pt(it), cuts.chi2MaxPt); | ||
| //float chi2Cut = cuts.chi2Scale * | ||
| // (cuts.chi2Coeff[0] + pt * (cuts.chi2Coeff[1] + pt * (cuts.chi2Coeff[2] + pt * cuts.chi2Coeff[3]))); |
There was a problem hiding this comment.
it is relevant as TRK-POG (or PF) may reintroduce the parametrization
thanks for clarifying here,
@czangela
please add a comment in the source code
you may need to click on |
|
Yes, because I rolled back 3 commits, but I marked these issues resolved before that. |
Do I understand correctly that you are proposing to make a separate technical PR to address the remaining issues? Please comment on #34286 (comment) |
|
I do not think there is any "bug" in the code as is. |
I don't think that is a bug; I just didn't know what I was talking about, and the comment is incorrect. |
| g.yfact[kk] *= dety; | ||
| } | ||
| // sample y in angle | ||
| float ys = 8.f - 4.f; // apperent bias of half pixel (see plot) |
There was a problem hiding this comment.
the one I presented one year ago
| // compute cotanbeta and use it to recompute error | ||
| dp.frame.rotation().multiply(dx, dy, dz, ux, uy, uz); | ||
| auto cb = std::abs(uy / uz); | ||
| int bin = int(cb * (285.f / 150.f) * 8.f) - 4; |
There was a problem hiding this comment.
can the 285 and 150 used here and elsewhere be taken from the DB (or at least be named constants)?
This will be "broken" for phase-2.
There was a problem hiding this comment.
this will NOT be used in phase2!
There was a problem hiding this comment.
the whole parametrization has no meaning outside Phase1
I am OK with it, and with the fact that a follow-up PR is needed to tune the chi2. |
|
+reconstruction
|
|
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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
cmsbuild
added
fully-signed
reconstruction-approved
and removed
pending-signatures
reconstruction-pending
labels
|
+1 |
Validation plots/RelValTTbar_14TeV/CMSSW_11_3_0_pre5-PU_113X_mcRun3_2021_realistic_v7-v1/GEN-SIM-DIGI-RAW
/RelValZMM_14/CMSSW_11_3_0_pre5-113X_mcRun3_2021_realistic_v7-v1/GEN-SIM-DIGI-RAW
Validation plots (CPU vs GPU)/RelValTTbar_14TeV/CMSSW_11_3_0_pre5-PU_113X_mcRun3_2021_realistic_v7-v1/GEN-SIM-DIGI-RAW
/RelValZMM_14/CMSSW_11_3_0_pre5-113X_mcRun3_2021_realistic_v7-v1/GEN-SIM-DIGI-RAW
Throughput plots/EphemeralHLTPhysics1/Run2018D-v1/RAW run=323775 lumi=53logs and
|
PR description:
By @VinInn at cms-patatrack/cmssw #528
This PR improves the error estimation in CPEFast used on GPU providing a simple and fast parameterization in terms of charge, position in x and size in y.
The result is very close to what CPEGeneric provides w/o track-angle (actually a bit more accurate thanks to the estimation of the y-error using the cluster size in y (itself improved with information from the charge unbalance).
It includes also code to recompute y-error using the track-able prior to fit (off by default)
The main effect is a reduction and flattening of the chi2 and a reduction of the tails of the pulls.
Given this improvements I have (for the the being) simplified the cut in chi2 to a single value.
In my opinion further tuning of eff vs fakerate shall be implemented elsewhere (eventually in client code)
It would be also useful to test it against some Simulation with "realistic" APE (such those in the "Legacy Reprocessing")
PR validation:
At http://aczirkos.web.cern.ch/aczirkos/cpefast_wo_track_angle_10_06_2021/new/
Summary
plots/plots_pixel_pixelHP/effandfakePtEtaPhiPrerequisites
Run on a GPU equipped machine. This validation was run on the cmg-gpu1080 machine, see this website for more information.
before
Benchmark release:
CMSSW_12_0_X_2021-06-22-2300after
branch cpefast_wo_track_angle_30_06_2021
The validation compares these two versions, which have the labels before and after in the plots.
Validation
2018
Dataset, filelist:
Steps 3 and 4
2021
Dataset, filelist:
Steps 3 and 4
2024
Dataset, filelist:
Steps 3 and 4
Compare DQM files
Use http://musich.web.cern.ch/musich/forAdinda/multi_loopAndPlot.C
Open interactive root session and compile file
root -b .L multi_LoopAndPlot.C++ multi_loopAndPlot("file1.root=before,file2.root=after,false)MTV:
if this PR is a backport please specify the original PR and why you need to backport that PR: