Skip to content

Conversation

@aloeliger
Copy link
Contributor

PR description:

This PR modifies the function of the last20Mismatches monitoring element to not include duplicate event mismatch reports on the Y-axis, instead just updating the types of mismatches seen for a given combination of Run, Lumi, and Event. As well, empty Y-axis labels are now simply given their corresponding bin number, to prevent empty string labels causing issues as seen before in #32250. This PR fixes the issue first spotted on this hypernews thread: https://hypernews.cern.ch/HyperNews/CMS/get/tier0-Ops/2192.html

PR validation:

All code compiles, and produces expected plots as seen when using runTheMatrix.py. Using @silviodonato 's method to test the DQM merging step, this code eliminates the merging error seen.

if this PR is a backport please specify the original PR and why you need to backport that PR:

This PR is a forward port to the master branch / 11_3_X of #32815. It was created by git cherry-pick -ing the three relevant commits on top of the current state of the master branch. This fix needs to be ported to be available in future production instances of CaloL1 DQM code.

@silviodonato
Copy link
Contributor

please test

@silviodonato
Copy link
Contributor

urgent

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 4, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32816/21002

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 4, 2021

A new Pull Request was created by @aloeliger for master.

It involves the following packages:

DQM/L1TMonitor

@andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @jfernan2, @rvenditti can you please review it and eventually sign? Thanks.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 4, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-727477/12711/summary.html
COMMIT: a48ba55
CMSSW: CMSSW_11_3_X_2021-02-03-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/32816/12711/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: 37
  • DQMHistoTests: Total histograms compared: 2752926
  • DQMHistoTests: Total failures: 10
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2752894
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 156 log files, 37 edm output root files, 37 DQM output files

@silviodonato
Copy link
Contributor

Backport #32815 already merged at the commit a48ba55 ( fbad4f6 )

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32816/21022

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2021

Pull request #32816 was updated. @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please check and sign again.

@aloeliger
Copy link
Contributor Author

aloeliger commented Feb 5, 2021

@silviodonato I think I have located the issue, a missing "&" (apologies it took me some time to understand and locate this), and the 25202.0 workflow plots look fixed in my test. Is there still in time to add this commit into 11_2_X and the MWGR?

@jfernan2
Copy link
Contributor

jfernan2 commented Feb 5, 2021

please test

@jfernan2
Copy link
Contributor

jfernan2 commented Feb 5, 2021

@silviodonato I think I have located the issue, a missing "&" (apologies it took me some time to understand and locate this), and the 25202.o workflow plots look fixed in my test. Is there still in time to add this commit into 11_2_X and the MWGR?

We can add the fix in 11_2_X at Online DQM at P5 for the GUI used by the shifters, as long as we have a PR, but I understand the fix affects mainly Offline for Tier0

@aloeliger
Copy link
Contributor Author

@silviodonato I think I have located the issue, a missing "&" (apologies it took me some time to understand and locate this), and the 25202.o workflow plots look fixed in my test. Is there still in time to add this commit into 11_2_X and the MWGR?

We can add the fix in 11_2_X at Online DQM at P5 for the GUI used by the shifters, as long as we have a PR, but I understand the fix affects mainly Offline for Tier0

@jfernan2, should I open up a new pull request to 11_2_X with this latest commit?

@jfernan2
Copy link
Contributor

jfernan2 commented Feb 5, 2021

should I open up a new pull request to 11_2_X with this latest commit?
Yes please

@aloeliger
Copy link
Contributor Author

Okay, I have opened a new PR to CMSSW_11_2_X with this fix as well.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-727477/12729/summary.html
COMMIT: 10d70eb
CMSSW: CMSSW_11_3_X_2021-02-04-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/32816/12729/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: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2751765
  • DQMHistoTests: Total failures: 15
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2751727
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 156 log files, 37 edm output root files, 37 DQM output files

@silviodonato
Copy link
Contributor

@aloeliger
in wf136.731 there are still some differences

before
image
https://tinyurl.com/y3623lfz

and after

image
https://tinyurl.com/y5jvovou

@aloeliger
Copy link
Contributor Author

aloeliger commented Feb 5, 2021

@aloeliger
in wf136.731 there are still some differences

This difference seems to be what was expected.

I have removed duplicate events (and therefore labels) from the Y-axis. The PR version does not have duplicate events shown on the Y-axis, instead only reporting each one once, alongside all types of mismatches seen, instead of reporting every mismatch individually.

The current version shows multiple copies of Run 274199, Lumi 21, Event 39733340, alongside multiple reports of the same kind of mismatch.

Comparing between the two, it does not seem like the current PR version of this monitoring element has missed any kinds of mismatches (the previous problem seen on 25202.0), nor does it seem to be reporting mismatches that were not seen.

@jfernan2
Copy link
Contributor

jfernan2 commented Feb 5, 2021

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2021

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)

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit f1a824a into cms-sw:master Feb 5, 2021
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.

4 participants