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

Remove unused vars in multiple pkgs #21221

Merged
merged 7 commits into from Nov 24, 2017
Merged

Conversation

mrodozov
Copy link
Contributor

@mrodozov mrodozov commented Nov 8, 2017

Remove unused vars from three packages. Note that the changes here:
https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/Utilities/src/LumiReWeighting.cc#L169
and here
https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/Utilities/src/LumiReWeighting.cc#L221
are leaving the for loops without obvious purpose (from a glance) yet there was no comp warning for PConf being unused (not sure why), so I'll leave them (the loops) as they are.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2017

The code-checks are being triggered in jenkins.

@mrodozov
Copy link
Contributor Author

mrodozov commented Nov 8, 2017

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2017

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-21221/1875

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/24283/console Started: 2017/11/08 19:15

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2017

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

It involves the following packages:

L1Trigger/L1TCalorimeter
PhysicsTools/Utilities
Validation/Geometry

@civanch, @Dr15Jones, @vazzolini, @kmaeshima, @ianna, @mdhildreth, @dmitrijus, @cmsbuild, @rekovic, @jfernan2, @vanbesien, @monttj, @mulhearn can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @kreczko, @rovere this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

@@ -166,8 +166,6 @@ double LumiReWeighting::weight( const edm::EventBase &e ) {

for(; PHist_iter<PHist.end() ;++PHist_iter) {
edm::ProcessConfiguration PConf = *(PHist_iter);
const edm::ReleaseVersion& Release = PConf.releaseVersion() ;
Copy link
Contributor

Choose a reason for hiding this comment

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

@mdhildreth - please, have a look. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

indeed, these loops appear to make no sense..

Copy link
Contributor

Choose a reason for hiding this comment

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

With this change PConf is now an unused variable.

I checked, and non of the functions called in this loop have side-effects, so there is no useful work being done.

On the other hand, the call to e.processHistory() can throw if there is a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm updating the PR then

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2017

Comparison job queued.

@@ -166,8 +166,6 @@ double LumiReWeighting::weight( const edm::EventBase &e ) {

for(; PHist_iter<PHist.end() ;++PHist_iter) {
edm::ProcessConfiguration PConf = *(PHist_iter);
const edm::ReleaseVersion& Release = PConf.releaseVersion() ;
Copy link
Contributor

Choose a reason for hiding this comment

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

With this change PConf is now an unused variable.

I checked, and non of the functions called in this loop have side-effects, so there is no useful work being done.

On the other hand, the call to e.processHistory() can throw if there is a problem.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2017

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

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /build/cmsbld/jenkins-workarea/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-21221/21234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 27
  • DQMHistoTests: Total histograms compared: 2967244
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2967065
  • DQMHistoTests: Total skipped: 178
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram bins added: 0.0 ( 23 files compared)
  • Checked 111 log files, 8 edm output root files, 27 DQM output files

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-21221/2140

@cmsbuild
Copy link
Contributor

Pull request #21221 was updated. @civanch, @Dr15Jones, @vazzolini, @kmaeshima, @ianna, @mdhildreth, @dmitrijus, @cmsbuild, @rekovic, @jfernan2, @vanbesien, @monttj, @mulhearn can you please check and sign again.

@davidlange6
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 22, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/24603/console Started: 2017/11/22 12:43

@cmsbuild
Copy link
Contributor

@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-21221/24603/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 27
  • DQMHistoTests: Total histograms compared: 2833444
  • DQMHistoTests: Total failures: 21
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2833245
  • DQMHistoTests: Total skipped: 178
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1.18999999993 KiB( 23 files compared)
  • Checked 111 log files, 8 edm output root files, 27 DQM output files

@ianna
Copy link
Contributor

ianna commented Nov 22, 2017

+1

@davidlange6
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 8c36002 into cms-sw:master Nov 24, 2017
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