Improvements to the `SiStripHitEfficiencyHarvester` for Run 3 data-taking #38592

mmusich · 2022-07-04T13:58:36Z

PR description:

As per request of the SiStrip offline group, I apply few improvement for the harvesting step:

in case the harvester is NOT run at PCL (flag isAtPCL set to false) produce the final good module/ bad module TGraphError plot for the hit efficiency;
in case the harvester is NOT run at PCL (flag isAtPCL set to false) produce a TTree to store per-DetId information for further post-processing from experts / shifters;
introduced a SiStripHitEffData struct to store the per event FEDErrors and some events statistics;
profited of the PR to clean-up slightly also SiStripHitEffFromCalibTree: used lower case to name methods and use usesResource(TFileService::kSharedResource) for threading efficiency.

PR validation:

Run the following commands:

cmsDriver.py stepReAlCa -s ALCA:PromptCalibProdSiStripHitEff --conditions 123X_dataRun2_v2 --scenario pp --data --era Run2_2018 --datatier ALCARECO --eventcontent ALCARECO --processName=ReAlCa -n 100000 --dasquery='file dataset=/StreamExpress/Run2018D-SiStripCalMinBias-Express-v1/ALCARECO run=325172' --nThreads=4

followed by:

cmsDriver.py stepHarvest -s ALCAHARVEST:SiStripHitEff --conditions 123X_dataRun2_v2 --scenario pp --data --era Run2_2018 --filein file:PromptCalibProdSiStripHitEfficiency.root -n -1

and obtained the following plot:

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:

Not a backport but needs to be backported for the data-taking.

mmusich · 2022-07-04T13:59:02Z

hold

still some work is needed before merge, but I am glad to start taking comments.

cmsbuild · 2022-07-04T14:07:28Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38592/30848

This PR adds an extra 56KB to repository

cmsbuild · 2022-07-04T14:07:52Z

Pull request has been put on hold by @mmusich
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

cmsbuild · 2022-07-04T14:07:56Z

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

CalibTracker/SiStripHitEfficiency (alca)
CommonTools/ConditionDBWriter (db)

@malbouis, @yuanchao, @cmsbuild, @ggovi, @francescobrivio, @ChrisMisan, @tvami can you please review it and eventually sign? Thanks.
@echabert, @pieterdavid, @robervalwalsh, @gbenelli, @tocheng, @mmusich this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

tvami · 2022-07-04T16:59:33Z

CalibTracker/SiStripHitEfficiency/plugins/SiStripHitEfficiencyHarvester.cc

-          tkMapBad.fillc(det, 255, 255, 255);
+      // use only the "good" modules
+      if (stripQuality_->getBadApvs(det) == 0) {
+        const auto eff = num / denom;


if the auto for the num and denom is an uint, will the auto for the eff somehow take care of the integer division?

The code was already like that before, and as far as I can tell does the right thing.

CalibTracker/SiStripHitEfficiency/plugins/SiStripHitEfficiencyHarvester.cc

CalibTracker/SiStripHitEfficiency/plugins/SiStripHitEfficiencyWorker.cc

mmusich · 2022-07-05T08:26:08Z

please test

tvami · 2022-07-05T09:24:54Z

I see this in the logs

In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-07-04-2300/src/DQMServices/Core/interface/MonitorElement.h:9,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-07-04-2300/src/DQMServices/Core/interface/DQMStore.h:4,
                 from src/CalibTracker/SiStripHitEfficiency/interface/SiStripHitEffData.h:4:
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-07-04-2300/src/DQMServices/Core/interface/DQMNet.h:4:10: fatal error: classlib/iobase/Socket.h: No such file or directory
    4 | #include "classlib/iobase/Socket.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

cmsbuild · 2022-07-05T12:52:25Z

-1

Failed Tests: HeaderConsistency RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-05d064/25977/summary.html
COMMIT: 78fdf32
CMSSW: CMSSW_12_5_X_2022-07-04-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38592/25977/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3654771
DQMHistoTests: Total failures: 41
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3654708
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 134.99 KiB( 49 files compared)
DQMHistoSizes: changed ( 1001.0 ): 134.990 KiB AlCaReco/SiStripHitEfficiency
Checked 208 log files, 45 edm output root files, 50 DQM output files
TriggerResults: no differences found

cmsbuild · 2022-07-05T13:52:34Z

Pull request #38592 was updated. @malbouis, @yuanchao, @cmsbuild, @ggovi, @francescobrivio, @ChrisMisan, @tvami can you please check and sign again.

mmusich · 2022-07-05T14:15:39Z

please test

cmsbuild · 2022-07-05T18:03:44Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-05d064/25987/summary.html
COMMIT: 9b82ea6
CMSSW: CMSSW_12_5_X_2022-07-05-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38592/25987/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3654771
DQMHistoTests: Total failures: 41
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3654708
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 134.99 KiB( 49 files compared)
DQMHistoSizes: changed ( 1001.0 ): 134.990 KiB AlCaReco/SiStripHitEfficiency
Checked 208 log files, 45 edm output root files, 50 DQM output files
TriggerResults: no differences found

mmusich · 2022-07-09T14:07:48Z

unhold

some more work would still be desireable, but I can't follow-up quickly on this, while the current changes are needed for operations, so I won't hold it up further.

tvami · 2022-07-09T14:22:16Z

+1

diffs seen are in the Strips ALCARECO folder, and are expected: https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_12_5_X_2022-07-05-1100+05d064/51404/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVDSIPIXELCALRUN1+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4+ALCAHARVD5+ALCAHARVD7+ALCAHARVD8/AlCaReco_SiStripHitEfficiency.html
PR is in line with the describtion

cmsbuild · 2022-07-09T14:22:35Z

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. @perrotta, @dpiparo, @qliphy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

tvami · 2022-07-09T14:22:48Z

while the current changes are needed for operations, so I won't hold it up further.

Does this deserve an urgent label then @mmusich ?

mmusich · 2022-07-09T14:24:42Z

Does this deserve an urgent label then

I wouldn't say it's urgent, as far as I know the hit efficiency measurement is still being done with calibration trees anyway, so this will become relevant once the switchover is done in the shifter tools.

tvami · 2022-07-09T14:27:22Z

Ok, so not necessarily needed for 12_4_3

perrotta · 2022-07-09T14:36:35Z

+1

cmsbuild added this to the CMSSW_12_5_X milestone Jul 4, 2022

cmsbuild added alca-pending code-checks-pending db-pending orp-pending pending-signatures tests-pending labels Jul 4, 2022

cmsbuild added the hold label Jul 4, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 4, 2022

tvami reviewed Jul 4, 2022

View reviewed changes

CalibTracker/SiStripHitEfficiency/plugins/SiStripHitEfficiencyHarvester.cc Outdated Show resolved Hide resolved

tvami reviewed Jul 4, 2022

View reviewed changes

CalibTracker/SiStripHitEfficiency/plugins/SiStripHitEfficiencyWorker.cc Outdated Show resolved Hide resolved

cmsbuild added tests-started and removed tests-pending labels Jul 5, 2022

cmsbuild added tests-rejected and removed tests-started labels Jul 5, 2022

mmusich added 4 commits July 5, 2022 15:24

improve method naming scheme and use SharedResources

252e7be

add makeSummary plots for SiStripHitEfficiency workflow

b74b013

add SiStripHitEffData

44999ca

add summary tree production in SiStripHitEfficiencyHarvester

9b82ea6

mmusich force-pushed the sistripHitEffMiscellaneaImprovements branch from 78fdf32 to 9b82ea6 Compare July 5, 2022 13:44

cmsbuild removed the tests-rejected label Jul 5, 2022

cmsbuild added the code-checks-approved label Jul 5, 2022

cmsbuild added tests-started and removed tests-pending labels Jul 5, 2022

cmsbuild added tests-approved and removed tests-started labels Jul 5, 2022

mmusich marked this pull request as ready for review July 9, 2022 14:06

cmsbuild removed the hold label Jul 9, 2022

cmsbuild added alca-approved db-approved fully-signed and removed alca-pending db-pending pending-signatures labels Jul 9, 2022

cmsbuild added orp-approved and removed orp-pending labels Jul 9, 2022

cmsbuild merged commit 6f4c6e1 into cms-sw:master Jul 9, 2022

mmusich mentioned this pull request Jul 9, 2022

[12.4.X] Improvements to the SiStripHitEfficiencyHarvester for Run 3 data-taking #38671

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements to the `SiStripHitEfficiencyHarvester` for Run 3 data-taking #38592

Improvements to the `SiStripHitEfficiencyHarvester` for Run 3 data-taking #38592

mmusich commented Jul 4, 2022

mmusich commented Jul 4, 2022 •

edited

cmsbuild commented Jul 4, 2022

cmsbuild commented Jul 4, 2022

cmsbuild commented Jul 4, 2022

tvami Jul 4, 2022

mmusich Jul 4, 2022

mmusich commented Jul 5, 2022

tvami commented Jul 5, 2022

cmsbuild commented Jul 5, 2022

cmsbuild commented Jul 5, 2022

mmusich commented Jul 5, 2022

cmsbuild commented Jul 5, 2022

mmusich commented Jul 9, 2022

tvami commented Jul 9, 2022

cmsbuild commented Jul 9, 2022

tvami commented Jul 9, 2022

mmusich commented Jul 9, 2022

tvami commented Jul 9, 2022

perrotta commented Jul 9, 2022

Improvements to the SiStripHitEfficiencyHarvester for Run 3 data-taking #38592

Improvements to the SiStripHitEfficiencyHarvester for Run 3 data-taking #38592

Conversation

mmusich commented Jul 4, 2022

PR description:

PR validation:

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:

mmusich commented Jul 4, 2022 • edited

cmsbuild commented Jul 4, 2022

cmsbuild commented Jul 4, 2022

cmsbuild commented Jul 4, 2022

tvami Jul 4, 2022

Choose a reason for hiding this comment

mmusich Jul 4, 2022

Choose a reason for hiding this comment

mmusich commented Jul 5, 2022

tvami commented Jul 5, 2022

cmsbuild commented Jul 5, 2022

RelVals-INPUT

Comparison Summary

cmsbuild commented Jul 5, 2022

mmusich commented Jul 5, 2022

cmsbuild commented Jul 5, 2022

Comparison Summary

mmusich commented Jul 9, 2022

tvami commented Jul 9, 2022

cmsbuild commented Jul 9, 2022

tvami commented Jul 9, 2022

mmusich commented Jul 9, 2022

tvami commented Jul 9, 2022

perrotta commented Jul 9, 2022

Improvements to the `SiStripHitEfficiencyHarvester` for Run 3 data-taking #38592

Improvements to the `SiStripHitEfficiencyHarvester` for Run 3 data-taking #38592

mmusich commented Jul 4, 2022 •

edited