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

GEM online DQM bug fix - full GE1/1, backport to 11_0_X #28977

Merged
merged 7 commits into from Apr 16, 2020

Conversation

quark2
Copy link
Contributor

@quark2 quark2 commented Feb 17, 2020

PR description:

This backport PR to 11_0_X which fixes a bug on the part of onlineDQM for GEM. The previous version uses endRun to draw summary plots, which is not a good idea in online DQM. This new version moves it to dqmEndLuminosityBlock.

PR validation:

Test are done and one can check again by runTheMatrix workflows

@jshlee @watson-ij

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 17, 2020

A new Pull Request was created by @quark2 for CMSSW_11_0_X.

It involves the following packages:

DQM/GEM

@andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks.
@davidlange6, @silviodonato, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 17, 2020

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4709/console Started: 2020/02/17 18:37

@andrius-k
Copy link

Hi quark2,
Now it looks good for the Online, but in order for the harvester to be working in the Offline, you also have to provide the harvesting functionality in the endJob method.

@quark2
Copy link
Contributor Author

quark2 commented Feb 17, 2020

Hi @andrius-k,

Thanks for the check. And okay, I'll note that. Offline DQM is still in progress so that we will update endJob for it.

Best regards,
Byeonghak Ko

@jfernan2
Copy link
Contributor

backport #28978

@cmsbuild
Copy link
Contributor

+1
Tested at: 2104119
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-93f38f/4709/summary.html
CMSSW: CMSSW_11_0_X_2020-02-17-1100
SCRAM_ARCH: slc7_amd64_gcc820

@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-93f38f/4709/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2793840
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2793497
  • DQMHistoTests: Total skipped: 341
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
  • Checked 147 log files, 16 edm output root files, 34 DQM output files

@jfernan2
Copy link
Contributor

jfernan2 commented Feb 18, 2020

Hi @quark2 there are Clang warnings to fix:
DQM/GEM/plugins/GEMDQMHarvester.cc
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_0_X_2020-02-17-1100/src/DQM/GEM/plugins/GEMDQMHarvester.cc:31:8: warning: 'dqmEndLuminosityBlock' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]

@cmsbuild
Copy link
Contributor

Pull request #28977 was updated. @andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please check and sign again.

@quark2
Copy link
Contributor Author

quark2 commented Mar 30, 2020

Dear @jfernan2,

Okay, I'll note that. And the GT has been update. Also, some minor update is included. They are also patched to the PR to master.

Best regards,
Byeonghak Ko

@silviodonato
Copy link
Contributor

#28978 was integrated. @quark2 can you synchronize this backport to #28978?

@quark2
Copy link
Contributor Author

quark2 commented Apr 14, 2020

Hi @silviodonato,

The changes of this PR and #28978 already coincide (except the GT field which must be different) at least in the GEM onlineDQM part. Is there another thing I have to synchonize?

Best regards,
Byeonghak Ko

@silviodonato
Copy link
Contributor

@quark2 I thought @jfernan2 was waiting for an update of this PR.
@jfernan2 this PR is still on hold from you.

@jfernan2
Copy link
Contributor

unhold

@jfernan2
Copy link
Contributor

@silviodonato since we could not test the PR that's why held it, but I understand you want to merge it for the future.

@cmsbuild cmsbuild removed the hold label Apr 14, 2020
@jfernan2
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 14, 2020

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/5695/console Started: 2020/04/14 22:04

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_11_0_X IBs after it passes the integration tests and once validation in the development release cycle CMSSW_11_1_X is complete. This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2)

@cmsbuild
Copy link
Contributor

+1
Tested at: 886a600
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-93f38f/5695/summary.html
CMSSW: CMSSW_11_0_X_2020-04-14-1100
SCRAM_ARCH: slc7_amd64_gcc820

@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-93f38f/5695/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2793840
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2793497
  • DQMHistoTests: Total skipped: 341
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
  • Checked 147 log files, 16 edm output root files, 34 DQM output files

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 6fcf49f into cms-sw:CMSSW_11_0_X Apr 16, 2020
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

5 participants