-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[12.4.X] SiStripHitEfficiency
PCL workflow, take into account modules with FEDErrors
#39357
[12.4.X] SiStripHitEfficiency
PCL workflow, take into account modules with FEDErrors
#39357
Conversation
A new Pull Request was created by @mmusich (Marco Musich) for CMSSW_12_4_X. It involves the following packages:
@malbouis, @yuanchao, @cmsbuild, @saumyaphor4252, @francescobrivio, @ChrisMisan, @tvami can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
type bug-fix |
test parameters:
|
@cmsbuild, please test |
urgent
|
-1 Failed Tests: RelVals-INPUT RelVals-INPUT
Comparison Summary@slava77 comparisons for the following workflows were not done due to missing matrix map:
Summary:
|
@cmsbuild please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c10b19/27458/summary.html Comparison Summary@slava77 comparisons for the following workflows were not done due to missing matrix map:
Summary:
|
+alca
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_6_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
backport of #39351
PR description:
When using the DQM files produced by the
SiStripHitEfficiency
PCL in 2022 data, it was noticed that not all the modules that should be masked with theSiStripQuality
are effectively masked within the DQM workflow, when comparing with the traditional calibrations-tree based workflow.The issue has been traced down on the usage of the modules affected by
FEDErrors
in the computation of the efficiency.In PR #38592 a mechanism to count the
FEDErrors
per module (via commit 44999ca ) was introduced since at the harvesting step level it's not possible to access directly at the per-eventFEDErrors
edm collection. This entailed populating aTkHistoMap
with the occupancy of modules in FEDError per event, but this approach had two problems:endJob
transition ofStripHitEfficiencyWorker
(that in aDQMEDAnalyzer
is not run by the framework, resulting in empty maps cf https://tinyurl.com/2p5tdgqs)SiStripHitEfficiencyHarvester
such that those modules could be excluded from the list of inefficient ones.This PR solves the issue by changing the
SiStripHitEffData
ancillarystruct
and its usage in the classes of theSiStripHitEfficiency
PCL workflow. A somewhat arbitrary threshold of 75% of the events is used to flag modules that have problems with FEDErrors, permanently excluding them from the computation.Finally I profit of this to transform a relic thread-unsafe
cout
to the message logging system in commit dee0359.PR validation:
Run the following commands:
cmsDriver.py stepReAlCa -s ALCA:PromptCalibProdSiStripHitEff --conditions 124X_dataRun3_Express_v5 --scenario pp --data --era Run3 --datatier ALCARECO --eventcontent ALCARECO --processName=ReAlCa -n 100000 --dasquery='file dataset=/StreamExpress/Run2022D-SiStripCalMinBias-Express-v2/ALCARECO run=357898' --nThreads=4
followed by:
cmsDriver.py stepHarvest -s ALCAHARVEST:SiStripHitEff --conditions 124X_dataRun3_Express_v5 --scenario pp --data --era Run3 --filein file:PromptCalibProdSiStripHitEff.root -n -1
with this branch and without it and observed the following features:
FEDErrorTkDetMaps
folder now contains full plots (as can be checked at https://tinyurl.com/2z7nrg9a after logging withssh -NL 8060:localhost:8060 <USER>@lxplus724.cern.ch
)If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
Verbatim backport of #39351 to CMSSW_12_5_X (for data-taking purposes)