Merge ShallowGainCalibration logic in SiStripGainsPCLWorker and a minor bugfix #33523
Conversation
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33523/22282
|
|
A new Pull Request was created by @pieterdavid (Pieter David) for master. It involves the following packages: CalibTracker/SiStripChannelGain @malbouis, @yuanchao, @christopheralanwest, @cmsbuild, @tlampen, @pohsun, @francescobrivio can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild, please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-421d03/14574/summary.html Comparison SummarySummary:
|
cmsbuild
added
code-checks-pending
tests-pending
and removed
code-checks-approved
tests-approved
labels
|
@mmusich noticed a difference in the histograms that was not expected (cluster count in AlCaReco/SiStripGainsAAG/EventStats for 1001.0), but after some discussion we find it more logical to fill the actual number of clusters that are used for the calibration than the total number of clusters on all tracks - so with the last commit the difference in that bin is larger but expected (this histogram is not used downstream, the harvester takes everything from the 2D Charge_Vs_Index histogram); sorry for the extra noise. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33523/22289
|
|
Pull request #33523 was updated. @malbouis, @yuanchao, @christopheralanwest, @cmsbuild, @tlampen, @pohsun, @francescobrivio can you please check and sign again. |
|
test parameters:
|
|
@cmsbuild, please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-421d03/14579/summary.html Comparison Summary@slava77 comparisons for the following workflows were not done due to missing matrix map:
Summary:
|
|
+1
|
cmsbuild
added
alca-approved
fully-signed
and removed
alca-pending
pending-signatures
labels
|
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) |
|
+1 |
|
cc @cms-sw/trk-dpg |
|
cc @cms-sw/trk-dpg-l2 |
PR description:
To minimise the risk of diverging during data-taking, the SiStrip gains PCL reused the flat tree maker (ShallowGainCalibration), and read the branches it produced. We are reorganising the calibration tree code for run 3 (moving to the NanoAOD framework / ALCANANO, see this presentation, which will replace ShallowGainCalibration), so now it seems cleaner to remove the intermediate step (that brings the track and cluster selection logic together in the PCL worker, where most of it already was).
In the process, I found a bug in the old code that checks if a cluster crosses or touches an APV boundary (group of 128 strips), and fixed that (the cut removed a bit too many clusters, so the only effect is a small increase, of about 1%, in the number of clusters considered - all distributions should be compatible).
PR validation:
Compared workflow 1001.2, which contains the SiStrip gain calibration PCL. No changes before the last commit; with that change included a small increase in cluster statistics.
CC: @ataliercio @mdelcourt @robervalwalsh @mmusich