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

Updating HLT DQM and validation for Hgg paths #19612

Closed

Conversation

mplaner
Copy link
Contributor

@mplaner mplaner commented Jul 7, 2017

This is the 92X version of PR #19023

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 7, 2017

A new Pull Request was created by @mplaner for CMSSW_9_2_X.

It involves the following packages:

DQMOffline/Trigger
HLTriggerOffline/Higgs

@vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@battibass, @mtosi, @jhgoh, @calderona, @HuguesBrun, @trocino, @rociovilar this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

@mtosi
Copy link
Contributor

mtosi commented Jul 7, 2017

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 7, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21250/console Started: 2017/07/07 13:19

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 7, 2017

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 7, 2017

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 22
  • DQMHistoTests: Total histograms compared: 1815737
  • DQMHistoTests: Total failures: 29350
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1786221
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 90 log files, 14 edm output root files, 22 DQM output files

@dmitrijus
Copy link
Contributor

How can this be a backport if it contains three times as much as there are changes in #19023 ?

@mplaner
Copy link
Contributor Author

mplaner commented Jul 10, 2017

@dmitrijus As far as I can tell, the issue is that several other PRs which are currently in 93X don't exist in 92X. So, for instance, we have to add in the "new" DQMOffline/Trigger/plugins/PhotonMonitor.cc and DQMOffline/Trigger/plugins/PhotonMonitor.h.

But for the 93X PR, those files already existed and I simply had to add a few lines.

I'm not sure what the correct procedure should be. I opted to keep the changes as minimal as possible, but I included what I needed from the PR. So, they should only need to add in some python configs to run their DQM in 92X.

@dmitrijus
Copy link
Contributor

The correct and desirable procedure would be to find the PR (in 93x) which extracted those extra files. Once you find this PR, you backport that PR either together or separately from this. Maybe there is already a pull request which backports it too.

Copying files around is the worst you can do. This way you destroy the integrity of the PR which introduced them.

@mplaner
Copy link
Contributor Author

mplaner commented Jul 11, 2017

Okay, do we have some official documentation for this? My main hesitation is that the PRs contain code for DQM plots which I can't properly validate. If I submit the backport for PRs from others, I can't guarantee the code I didn't write is working correctly.

Is there a reasonable way to search through the pending PRs to see if these changes have been requested already?

@cmsbuild
Copy link
Contributor

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

@mplaner
Copy link
Contributor Author

mplaner commented Jul 11, 2017

@dmitrijus, I have found the only relevant PR and rebased with it (#19520). It wasn't in last night's integration build, but I think my rebasing should allow things to work smoothly.

Let me know if this doesn't follow best practices.

@dmitrijus
Copy link
Contributor

How did you rebase? should have been "git-cms-merge topic 19520" followed by your commits.
Ideally, maybe the best solution is to wait until #19520 is merged and then do the proper rebase.

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