Run2-alca109 Update a few plugins to conform standard #21720

bsunanda · 2017-12-14T16:40:37Z

Update IsolatedGenParticle and IsolatedTrackHcalScale used to get energy scale factor Hcal

cmsbuild · 2017-12-14T16:41:07Z

The code-checks are being triggered in jenkins.

cmsbuild · 2017-12-14T17:16:17Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-21720/2596

cmsbuild · 2017-12-14T17:16:58Z

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

It involves the following packages:

Calibration/IsolatedParticles

@ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @lpernie can you please review it and eventually sign? Thanks.
@ghellwig, @mmusich, @tocheng this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

davidlange6 · 2017-12-14T17:42:50Z

please test

cmsbuild · 2017-12-14T17:43:21Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/25103/console Started: 2017/12/14 19:19

cmsbuild · 2017-12-14T20:10:46Z

+1
Tested at: dff280d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21720/25103/summary.html

cmsbuild · 2017-12-14T20:10:49Z

Comparison job queued.

cmsbuild · 2017-12-14T22:07:29Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21720/25103/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-21720/1325.7_TTbar_13_94XNanoAODINPUT+TTbar_13_94XNanoAODINPUT+NANOEDMMC2017

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 17 differences found in the comparisons
DQMHistoTests: Total files compared: 27
DQMHistoTests: Total histograms compared: 2836825
DQMHistoTests: Total failures: 2770
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2833877
DQMHistoTests: Total skipped: 178
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 1.09000000004 KiB( 23 files compared)
Checked 113 log files, 9 edm output root files, 27 DQM output files

ghellwig · 2017-12-15T08:42:54Z

Calibration/IsolatedParticles/plugins/IsolatedGenParticles.cc

+  void   clearTreeVectors();
+  int    particleCode(int);
+
+  static const int     NPBins_   = 3;


@bsunanda you could even make them constexpr

ghellwig · 2017-12-15T08:45:22Z

Calibration/IsolatedParticles/plugins/IsolatedGenParticles.cc


 IsolatedGenParticles::IsolatedGenParticles(const edm::ParameterSet& iConfig) {

-  genSrc_    = iConfig.getUntrackedParameter("GenSrc",std::string("generatorSmeared"));
+  usesResource("TFileService");


please use the string constant:

usesResource(TFileService::kSharedResource);

ghellwig · 2017-12-15T08:46:03Z

Calibration/IsolatedParticles/plugins/IsolatedGenParticles.cc

+  a_Isolation_= iConfig.getUntrackedParameter<bool>("UseConeIsolation",false);
+  pCutIsolate_= iConfig.getUntrackedParameter<double>("PMaxIsolation",20.0);
+  verbosity_  = iConfig.getUntrackedParameter<int>("Verbosity", 0);
+  debugL1Info_= iConfig.getUntrackedParameter<bool>( "DebugL1Info", false );


this could go into the initializer list of the constructor

ghellwig · 2017-12-15T08:49:41Z

Calibration/IsolatedParticles/plugins/IsolatedTracksHcalScale.cc

-#include "Calibration/IsolatedParticles/plugins/IsolatedTracksHcalScale.h"
-
+// system include files
+#include <memory>


no need to include this if you do not use smart pointers

ghellwig · 2017-12-15T08:51:23Z

Calibration/IsolatedParticles/test/proto_runIsolatedGenParticles_cfg.py

-#process.isolatedGenParticles.PTMin             = 0.0
-process.isolatedGenParticles.Verbosity         = 0
-#process.isolatedGenParticles.MaxChargedHadronEta = 3.5
+process.load("Calibration.IsolatedParticles.IsolatedGenParticles_cfi")


I might be wrong, but I believe the CMS naming convention is to start with lower case.

This is the name of the file created in cfipython

well, then you should adapt fillDescriptions accordingly

cmsbuild · 2017-12-15T22:14:06Z

The code-checks are being triggered in jenkins.

cmsbuild · 2017-12-15T22:16:08Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-21720/2624

cmsbuild · 2017-12-15T22:16:54Z

Pull request #21720 was updated. @ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @lpernie can you please check and sign again.

bsunanda · 2017-12-15T22:17:03Z

@cmsbuild Please test

cmsbuild · 2017-12-15T22:17:43Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/25133/console Started: 2017/12/15 23:18

cmsbuild · 2017-12-15T23:59:04Z

+1
Tested at: 8113c7d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21720/25133/summary.html

cmsbuild · 2017-12-15T23:59:07Z

Comparison job queued.

cmsbuild · 2017-12-16T02:24:12Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-21720/25133/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-21720/1325.7_TTbar_13_94XNanoAODINPUT+TTbar_13_94XNanoAODINPUT+NANOEDMMC2017

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 18 differences found in the comparisons
DQMHistoTests: Total files compared: 27
DQMHistoTests: Total histograms compared: 2836825
DQMHistoTests: Total failures: 1391
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2835256
DQMHistoTests: Total skipped: 178
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 1.58000000015 KiB( 23 files compared)
Checked 113 log files, 9 edm output root files, 27 DQM output files

ghellwig · 2017-12-18T08:12:55Z

+1

cmsbuild · 2017-12-18T08:13:20Z

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 will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

davidlange6 · 2017-12-18T09:01:04Z

+1

Update a few plugins to conform standard

dff280d

cmsbuild added this to the CMSSW_10_0_X milestone Dec 14, 2017

cmsbuild added alca-pending code-checks-pending comparison-pending orp-pending pending-signatures tests-pending labels Dec 14, 2017

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 14, 2017

cmsbuild added tests-started and removed tests-pending labels Dec 14, 2017

cmsbuild added tests-approved and removed tests-started labels Dec 14, 2017

cmsbuild added comparison-available and removed comparison-pending labels Dec 14, 2017

ghellwig suggested changes Dec 15, 2017

View reviewed changes

Modify as suggested by reviewer

04cd9b5

cmsbuild added code-checks-pending and removed code-checks-approved comparison-available tests-approved labels Dec 15, 2017

cmsbuild added code-checks-pending comparison-pending tests-pending and removed tests-approved labels Dec 15, 2017

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 15, 2017

cmsbuild added tests-started and removed tests-pending labels Dec 15, 2017

cmsbuild added tests-approved and removed tests-started labels Dec 15, 2017

cmsbuild added comparison-available and removed comparison-pending labels Dec 16, 2017

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels Dec 18, 2017

cmsbuild added orp-approved and removed orp-pending labels Dec 18, 2017

cmsbuild merged commit d13bef2 into cms-sw:master Dec 18, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run2-alca109 Update a few plugins to conform standard #21720

Run2-alca109 Update a few plugins to conform standard #21720

bsunanda commented Dec 14, 2017

cmsbuild commented Dec 14, 2017

cmsbuild commented Dec 14, 2017

cmsbuild commented Dec 14, 2017

davidlange6 commented Dec 14, 2017

cmsbuild commented Dec 14, 2017 •

edited

cmsbuild commented Dec 14, 2017

cmsbuild commented Dec 14, 2017

cmsbuild commented Dec 14, 2017

ghellwig Dec 15, 2017

bsunanda Dec 15, 2017

ghellwig Dec 15, 2017

bsunanda Dec 15, 2017

ghellwig Dec 15, 2017

bsunanda Dec 15, 2017

ghellwig Dec 15, 2017

bsunanda Dec 15, 2017

ghellwig Dec 15, 2017

bsunanda Dec 15, 2017

ghellwig Dec 15, 2017

cmsbuild commented Dec 15, 2017

cmsbuild commented Dec 15, 2017

cmsbuild commented Dec 15, 2017

bsunanda commented Dec 15, 2017

cmsbuild commented Dec 15, 2017 •

edited

cmsbuild commented Dec 15, 2017

cmsbuild commented Dec 15, 2017

cmsbuild commented Dec 16, 2017

ghellwig commented Dec 18, 2017

cmsbuild commented Dec 18, 2017

davidlange6 commented Dec 18, 2017

Run2-alca109 Update a few plugins to conform standard #21720

Run2-alca109 Update a few plugins to conform standard #21720

Conversation

bsunanda commented Dec 14, 2017

cmsbuild commented Dec 14, 2017

cmsbuild commented Dec 14, 2017

cmsbuild commented Dec 14, 2017

davidlange6 commented Dec 14, 2017

cmsbuild commented Dec 14, 2017 • edited

cmsbuild commented Dec 14, 2017

cmsbuild commented Dec 14, 2017

cmsbuild commented Dec 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Dec 15, 2017

cmsbuild commented Dec 15, 2017

cmsbuild commented Dec 15, 2017

bsunanda commented Dec 15, 2017

cmsbuild commented Dec 15, 2017 • edited

cmsbuild commented Dec 15, 2017

cmsbuild commented Dec 15, 2017

cmsbuild commented Dec 16, 2017

ghellwig commented Dec 18, 2017

cmsbuild commented Dec 18, 2017

davidlange6 commented Dec 18, 2017

cmsbuild commented Dec 14, 2017 •

edited

cmsbuild commented Dec 15, 2017 •

edited