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.

Sign up for GitHub

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

Jump to bottom

gcc7 compilation warnings fix in RecoJets pkgs #19924

Merged
merged 1 commit into from Jul 31, 2017
Merged

gcc7 compilation warnings fix in RecoJets pkgs #19924

merged 1 commit into from Jul 31, 2017

Conversation

mrodozov
Copy link
Contributor

Fixes for compilation warnings when compiling with gcc7 and more flags listed here:
https://cmssdt.cern.ch/SDT/cgi-bin/showBuildLogs.py/slc6_amd64_gcc700/www/mon/9.3-mon-23/CMSSW_9_3_X_2017-07-24-2300
packages 18 and 19 comp warnings

@cmsbuild cmsbuild added this to the CMSSW_9_3_X milestone Jul 26, 2017
@cmsbuild
Copy link
Contributor

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

It involves the following packages:

RecoJets/FFTJetProducers
RecoJets/JetAnalyzers

@perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@TaiSakuma, @ahinzmann, @rappoccio, @jdolen, @nhanvtran, @gkasieczka, @schoef, @mariadalfonso, @seemasharmafnal this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

perrotta
perrotta reviewed Jul 26, 2017
View reviewed changes
RecoJets/FFTJetProducers/plugins/FFTJetEFlowSmoother.cc Outdated
@@ -34,8 +34,8 @@
#include "FWCore/ParameterSet/interface/ParameterSet.h"

#include "DataFormats/Common/interface/View.h"
#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"

//#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove commented out code, if there is no reason to keep it

RecoJets/JetAnalyzers/src/FFTJetImageRecorder.cc Outdated
@@ -33,8 +33,9 @@
#include "FWCore/ParameterSet/interface/ParameterSet.h"

#include "DataFormats/Common/interface/Handle.h"
#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"

//#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove commented out code, if there is no reason to keep it

RecoJets/JetAnalyzers/src/FFTJetPileupAnalyzer.cc Outdated
@@ -31,7 +31,9 @@
#include "FWCore/ParameterSet/interface/ParameterSet.h"

#include "DataFormats/Common/interface/Handle.h"
#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
//#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove commented out code, if there is no reason to keep it

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 26, 2017 •
edited

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21785/console Started: 2017/07/26 15:43

@cmsbuild
Copy link
Contributor

+1
Tested at: 6063574
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19924/21785/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
cc98e01
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19924/21785/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19924/21785/git-merge-result

@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-19924/21785/summary.html

Comparison Summary:

  • You potentially added 5 lines to the logs
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 2363781
  • DQMHistoTests: Total failures: 29558
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2334057
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 93 log files, 14 edm output root files, 23 DQM output files

slava77
slava77 reviewed Jul 27, 2017
View reviewed changes
RecoJets/JetAnalyzers/src/FFTJetImageRecorder.cc Outdated
#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"

#include <TH3F.h>
#include <TH2D.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

is TH2D really necessary?
I only see TH3F mentioned in the file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't. The files are updated accordingly

@cmsbuild
Copy link
Contributor

Pull request #19924 was updated. @perrotta, @vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @rekovic, @slava77, @vanbesien, @mulhearn, @davidlange6 can you please check and sign again.

perrotta
perrotta reviewed Jul 28, 2017
View reviewed changes
RecoJets/JetAnalyzers/src/FFTJetPileupAnalyzer.cc Outdated
@@ -31,7 +31,8 @@
#include "FWCore/ParameterSet/interface/ParameterSet.h"

#include "DataFormats/Common/interface/Handle.h"
#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
#include <TH3F.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is TH3F really needed? I only see TH2D in the code

DQM/L1TMonitor/src/L1TRate.cc Outdated
@@ -24,7 +24,7 @@
#include "CondFormats/DataRecord/interface/L1GtTriggerMenuRcd.h"
#include "CondFormats/DataRecord/interface/L1GtPrescaleFactorsAlgoTrigRcd.h"

#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
//#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove commented out code, or add another line of comment which explains why it must be kept.

DQM/L1TMonitorClient/src/L1TOccupancyClient.cc Outdated
@@ -8,7 +8,7 @@
#include "DQMServices/Core/interface/QReport.h"
#include "DQMServices/Core/interface/DQMStore.h"
#include "DQMServices/Core/interface/MonitorElement.h"
#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
//#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove commented out code, or add another line of comment which explains why it must be kept.

DQM/L1TMonitorClient/src/L1TTestsSummary.cc Outdated
@@ -9,7 +9,7 @@
#include "DQMServices/Core/interface/DQMStore.h"
#include "DQMServices/Core/interface/MonitorElement.h"
#include "DQMServices/Core/interface/DQMChannel.h"
#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
//#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove commented out code, or add another line of comment which explains why it must be kept.

DQMOffline/L1Trigger/src/L1TMuonDQMOffline.cc Outdated
@@ -11,7 +11,7 @@
#include "DataFormats/L1TMuon/interface/RegionalMuonCandFwd.h"
#include "DataFormats/L1Trigger/interface/Muon.h"
#include "DQMServices/Core/interface/DQMStore.h"
#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
//#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove commented out code, or add another line of comment which explains why it must be kept.

DQMOffline/L1Trigger/src/L1TRate_Offline.cc Outdated
@@ -10,7 +10,7 @@
#include "CondFormats/DataRecord/interface/L1GtTriggerMenuRcd.h"
#include "CondFormats/DataRecord/interface/L1GtPrescaleFactorsAlgoTrigRcd.h"

#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
//#include "DataFormats/Histograms/interface/MEtoEDMFormat.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove commented out code, or add another line of comment which explains why it must be kept.

@davidlange6
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

@smuzaffar
Copy link
Contributor

smuzaffar commented Jul 28, 2017 •
edited

please test
last IB is broken which is now ignored for PR tests

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 28, 2017 •
edited

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21869/console Started: 2017/07/28 16:48

@cmsbuild
Copy link
Contributor

+1
Tested at: 6017761
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19924/21869/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
29a96fd
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19924/21869/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19924/21869/git-merge-result

@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-19924/21869/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: 39712
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2336656
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 93 log files, 14 edm output root files, 23 DQM output files

@slava77
Copy link
Contributor

slava77 commented Jul 31, 2017

+1

for #19924 6017761

  • technical update to avoid including DataFormats/Histograms/interface/MEtoEDMFormat.h which is not needed in full
  • jenkins tests pass

@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

+1

@cmsbuild cmsbuild merged commit 46f824c into cms-sw:master Jul 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@perrotta perrotta perrotta left review comments

@slava77 slava77 slava77 left review comments

Assignees
No one assigned
Labels
comparison-available fully-signed orp-approved reconstruction-approved tests-approved
Projects
None yet
Milestone
CMSSW_9_3_X
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@mrodozov @cmsbuild @perrotta @davidlange6 @smuzaffar @slava77