Skip to content
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

Monitoring of SiPixelQuality PCL using tracker offline DQM and bugfix for the SiPixelQuality tag for PromptReco #23616

Merged
merged 6 commits into from Jul 13, 2018

Conversation

tocheng
Copy link
Contributor

@tocheng tocheng commented Jun 18, 2018

Hello, the PR is meant to monitor the PCL payloads of SiPixelQuality using tracker offline DQM.
The harvester is changed to a DQMEDAnalyzer to produce the payloads and DQM IO in one CMSSW module.

The continuous payloads are replace by one payload if they are identical. This will clean the redundant payloads in the tags.

And with the help of the DQM plots, I found a bug in the tag for prompt-reco from the SiPixelQuality production; only the low DIGI ROCs in the permanent bad components were added to the tag for prompt-reco, while all the components that were low efficient (due to 2017 DCDC damage) were not included. So this bug was fixed in the later commit of the PR.

More details can be seen in the pixel offline meeting this week.
https://indico.cern.ch/event/688868/contributions/3062848/attachments/1680102/2698910/PixelQualityPCL_July3rd_2018_pixeloffline.pdf

Sorry for pending the PR for quite some time. I was trying to find way to normalize the DQM plots in the way that they can be easily compared between different runs regardless the length of the run. But the simple way led to the crash of all TProfile plots for tracker DQM. There are other solutions, but from my point of view, too complicated to be done in a reasonable time.
So I keep the plots as what they are and they are still good enough to use without the normalization.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23616/5240

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23616/5240/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23616/5240/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @tocheng (Tongguang) for master.

It involves the following packages:

CalibTracker/SiPixelQuality
Configuration/StandardSequences

@arunhep, @cerminar, @cmsbuild, @franzoni, @pohsun, @lpernie, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @GiacomoSguazzoni, @tocheng, @VinInn, @Martin-Grunewald, @dkotlins, @rovere, @mmusich, @dgulhan this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@arunhep
Copy link
Contributor

arunhep commented Jun 18, 2018

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 18, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28746/console Started: 2018/06/18 22:15

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23616/28746/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 2902013
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2901821
  • DQMHistoTests: Total skipped: 190
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
  • Checked 128 log files, 14 edm output root files, 31 DQM output files

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 6, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 6, 2018

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 6, 2018

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23616/29037/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 2899480
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2899289
  • DQMHistoTests: Total skipped: 190
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
  • Checked 128 log files, 14 edm output root files, 31 DQM output files

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 6, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 6, 2018

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 6, 2018

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23616/29038/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 2899480
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2899289
  • DQMHistoTests: Total skipped: 190
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
  • Checked 128 log files, 14 edm output root files, 31 DQM output files

@lpernie
Copy link
Contributor

lpernie commented Jul 8, 2018

+1

@@ -225,7 +225,7 @@
EcalPedestals = cms.Path(ALCAHARVESTEcalPedestals)
SiStripGainsAAG = cms.Path(ALCAHARVESTSiStripGainsAAG)
LumiPCC = cms.Path(ALCAHARVESTLumiPCC)
SiPixelQuality = cms.Path(ALCAHARVESTSiPixelQuality)
SiPixelQuality = cms.Path(ALCAHARVESTSiPixelQuality)#+siPixelPhase1DQMHarvester)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tocheng @lpernie this statement change is effectively doing nothing, I understand it is a placeholder left for the future, but what is the plan here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tocheng @lpernie @arunhep could you please clarify?

Copy link
Contributor Author

@tocheng tocheng Jul 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabiocos The original plan is to use the SiPixel Phase-I DQM Harvester class (as a template) to normalize the bincontent of the track map plots (in tracker DQM output) by the total number of lumi sections. So the bin content will represent the fraction of time that the ROC is bad in a run. (Then it will be easier to identify new ROCs that are bad for during whole run)
But I realize it needs a lot of customization as the functionalities I need is protected/guarded by the SiPixel DQM framework. I contacted the tracker DQM conveners but they are too busy to work on this special request.
So I decide to put this development for the future. The plots with binconent as the absolute number of lumi section is still good enough to monitor the payloads and identify new bad ROCs.
Currently siPixelPhase1DQMHarvester is dummy for the SiPixelQuality DQM production.
I commented it to prevent the impact from potential central SiPixel DQM development in the future.
I will discuss with the tracker DQM experts when the they have more time on it.
I don't think this development is very urgent or needed for this year.
Hopefully this clarifies the change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tocheng thank you for the detailed discussion. At this point anyway I need to understand whether this development is really needed in 10_2_0, as apparently stated by @lpernie @arunhep in the ORP spreadsheet, or it can stay on hold, as effectively it is dummy so far, as I get from your explanation

Copy link
Contributor Author

@tocheng tocheng Jul 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabiocos
I think I misunderstood your questions. "siPixelPhase1DQMHarvester" is dummy.
BUT The PR is NOT dummy. The implementation of ALCAHARVESTSiPixelQuality is bug-fixed and updated.

I think it is very critical for this PR to be merged into CMSSW_10_2_0.
Because it fixes a bug for the SiPixelQuality conditions to be used for prompt reconstruction : see da9f6e2

The DQM monitoring in the PR provides the feature to monitor the SiPixelQuality conditions produced by PCL and also to help offline tracker DQM shifter to find new bad components,
especially new bad components from DCDC converter breakdown
.

They are both very critical for the data taking and processing in CMSSW_10_2_0 release.

@tocheng
Copy link
Contributor Author

tocheng commented Jul 13, 2018

@fabiocos The motivation of this whole project is automation of lumibased pixel quality for prompt reco and monitoring of bad components due to DCDC converter breaking. Without the PR merged, neither of them can be realized.

@fabiocos
Copy link
Contributor

@tocheng, ok sorry for misinterpreting your answer.

@fabiocos
Copy link
Contributor

+operations

the update of StandardSequences is for the time being dummy, a placeholder for future deployment

@fabiocos
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

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 be automatically merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants