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

HLT DQM for 9.3.x - part 4 #19916

Merged
merged 88 commits into from
Jul 28, 2017
Merged

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Jul 25, 2017

Follow up to #19891 and #19912.

Merge the following HLT DQM pull requests (in this order):

and address the issues raised in the discussion of #19499, #19267.

bpfrancis and others added 30 commits June 29, 2017 15:52
…QM92X

Conflicts:
	DQMOffline/Trigger/python/ExoticaMonitoring_Client_cff.py
	DQMOffline/Trigger/python/ExoticaMonitoring_cff.py
@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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

hi @fwyzard - except for METplusTrackMonitor.cc this is ready (ok, modulo the strange TProfiles and # of bins that are known issues that i guess will be sorted out by dqm once all the prs go in).

so - if you want to remove that PR the rest can go - i'll comment on the one cc file when I get a bit more time.

@fwyzard
Copy link
Contributor Author

fwyzard commented Jul 27, 2017

hi @davidlange6,
if you are going to look at METplusTrackMonitor.cc I would suggest to look at #19917 rather than this one, as it includes some clean up form clang-tidy.

Would you be amenable to merging this PR is I just disable the use of METplusTrackMonitor at the python level, and apply any fixes you request in a follow up PR before re-enabling it ?
Unfortunately that was the first PR I merged here, so I would have to go through all the conflicts resolution again to redo this PR without METplusTrackMonitor :-(

Otherwise, I can redo the work if you prefer not to have the code in the release at all.

@cmsbuild
Copy link
Contributor

Pull request #19916 was updated. @vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @vanbesien, @davidlange6 can you please check and sign again.

@fwyzard
Copy link
Contributor Author

fwyzard commented Jul 27, 2017

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 27, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21840/console Started: 2017/07/27 20:46

@fwyzard
Copy link
Contributor Author

fwyzard commented Jul 27, 2017

This is my "quick" proposal in practice:

  • solve conflicts with master
  • disable all METplusTrackMonitor modules

.A

@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-19916/21840/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 2376534
  • DQMHistoTests: Total failures: 44870
  • DQMHistoTests: Total nulls: 300
  • DQMHistoTests: Total successes: 2331198
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 93 log files, 14 edm output root files, 23 DQM output files

@cmsbuild cmsbuild merged commit 2eb497d into cms-sw:master Jul 28, 2017
@fwyzard
Copy link
Contributor Author

fwyzard commented Jul 28, 2017

Thanks !

@fwyzard
Copy link
Contributor Author

fwyzard commented Jul 28, 2017

hi @davidlange6 , I've created #19946 to enable the MET + track trigger DQM.
If you have comments to METplusTrackMonitor.h / .cc can you post them there ?

@fwyzard fwyzard deleted the HLT_DQM_for_93x_part4 branch August 2, 2017 08:57
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

10 participants