New package DataFormats/L1TCalorimeterPhase2 for Phase2 L1T #29348

rekovic · 2020-03-31T00:00:22Z

PR description:

New package DataFormats/L1TCalorimeterPhase2 for Phase2 L1Trigger.

These classes are intended to hold L1T objects reconstructed basic stand-alone Calorimeter objects using full granularity of Phase2 calorimeters available to the Leve1 trigger in Phase2.

These objects can then be used downstream to reconstruct more complex objects, either through matching to L1TTTracks or in ParticleFlow algorithms.

To distinguish these objects from Phase-1 L1T objects, this package name contains string Phase2

There are existing DataFormats/L1TCalorimeter which hold Phase1-like calorimeter L1T objects reconstructed with 5x5 granularity available to the Level trigger in Phase1.

cmsbuild · 2020-03-31T00:00:50Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-03-31T00:05:17Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29348/14439

This PR adds an extra 20KB to repository

rekovic · 2020-03-31T00:05:30Z

please test

cmsbuild · 2020-03-31T00:05:38Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/5452/console Started: 2020/03/31 02:07

cmsbuild · 2020-03-31T00:05:39Z

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

It involves the following packages:

DataFormats/L1TCalorimeterPhase2

The following packages do not have a category, yet:

DataFormats/L1TCalorimeterPhase2
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@cmsbuild can you please review it and eventually sign? Thanks.
@rovere this is something you requested to watch as well.
@davidlange6, @silviodonato, @fabiocos you are the release manager for this.

cms-bot commands are listed here

rekovic · 2020-03-31T00:14:23Z

please test

cmsbuild · 2020-03-31T01:32:50Z

-1

Tested at: 8b8eeb4

CMSSW: CMSSW_11_1_X_2020-03-30-1100
SCRAM_ARCH: slc7_amd64_gcc820
You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5286f8/5452/summary.html

I found follow errors while testing this PR

Failed tests: HeaderConsistency

cmsbuild · 2020-03-31T01:32:53Z

Comparison job queued.

cmsbuild · 2020-03-31T08:10:10Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2692110
DQMHistoTests: Total failures: 35
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2691756
DQMHistoTests: Total skipped: 319
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

makortel · 2020-04-02T17:23:31Z

DataFormats/L1TCalorimeterPhase2/BuildFile.xml

+<use   name="rootrflx"/>
+<use   name="hepmc"/>
+<use   name="clhep"/>
+<use   name="SimDataFormats/GeneratorProducts"/>


By quick look boost, hepmc, clhep, and SimDataFormats/GeneratorProducts are not used. Can they be removed?

makortel · 2020-04-02T17:24:33Z

DataFormats/L1TCalorimeterPhase2/BuildFile.xml

@@ -0,0 +1,17 @@
+<flags   GENREFLEX_ARGS="--"/>


Is this flag needed? I don't see such a flag anywhere in CMSSW.

makortel · 2020-04-02T17:26:04Z

DataFormats/L1TCalorimeterPhase2/interface/L1CaloJet.h

+        : l1t::L1Candidate(p4), calibratedPt_(calibratedPt), hovere_(hovere), iso_(iso), PUcorrPt_(PUcorrPt){};
+
+    // For decay mode related checks with CaloTaus
+    std::vector<std::vector<float> > associated_l1EGs;


Member variables should be private.

makortel · 2020-04-02T17:27:24Z

DataFormats/L1TCalorimeterPhase2/interface/L1CaloJet.h

+    // For decay mode related checks with CaloTaus
+    std::vector<std::vector<float> > associated_l1EGs;
+
+    virtual ~L1CaloJet(){};


I wonder how this passed clang-tidy, I would have expected

Suggested change

virtual ~L1CaloJet(){};

~L1CaloJet() override {};

Actually the destructor can be removed (i.e. left for compiler to generate)

makortel · 2020-04-02T17:27:45Z

DataFormats/L1TCalorimeterPhase2/interface/L1CaloJet.h

+    inline float isolation() const { return iso_; };
+    inline float PUcorrPt() const { return PUcorrPt_; };
+    void SetExperimentalParams(const std::map<std::string, float>& params) { experimentalParams_ = params; };
+    const std::map<std::string, float> GetExperimentalParams() const { return experimentalParams_; };


Suggested change

const std::map<std::string, float> GetExperimentalParams() const { return experimentalParams_; };

const std::map<std::string, float>& experimentalParams() const { return experimentalParams_; };

to avoid a copy, and to remove get prefix of a getter (2.11 in http://cms-sw.github.io/cms_coding_rules.html)

makortel · 2020-04-02T17:46:37Z

DataFormats/L1TCalorimeterPhase2/interface/L1EGCrystalCluster.h

+      try {
+        return experimentalParams_.at(name);
+      } catch (...) {
+        std::cout << "Error: no mapping for ExperimentalParam: " << name << std::endl;


cout is not thread safe.

makortel · 2020-04-02T17:48:01Z

DataFormats/L1TCalorimeterPhase2/interface/L1EGCrystalCluster.h

+
+    // The index range depends on the algorithm eta,phi window, currently 3x5
+    // The pt should always be ordered.
+    inline float GetCrystalPt(unsigned int index) const {


Suggested change

inline float GetCrystalPt(unsigned int index) const {

inline float crystalPt(unsigned int index) const {

makortel · 2020-04-02T17:48:28Z

DataFormats/L1TCalorimeterPhase2/src/classes.h

+namespace {
+  namespace {
+
+    l1slhc::L1EGCrystalCluster egcrystalcluster;
+    std::vector<l1slhc::L1EGCrystalCluster> l1egcrystalclustervec;
+    l1slhc::L1EGCrystalClusterCollection l1egcrystalclustercoll;
+    edm::Wrapper<l1slhc::L1EGCrystalClusterCollection> wl1egcrystalclustercoll;
+
+    L1CaloTower l1CaloTower;
+    std::vector<L1CaloTower> l1CaloTowervec;
+    L1CaloTowerCollection l1CaloTowercoll;
+    edm::Wrapper<L1CaloTowerCollection> wl1CaloTowercoll;
+
+    l1slhc::L1CaloJet l1calojet;
+    std::vector<l1slhc::L1CaloJet> l1calojetvec;
+    l1slhc::L1CaloJetsCollection l1calojetcoll;
+    edm::Wrapper<l1slhc::L1CaloJetsCollection> wl1calojetcoll;
+
+  }  // namespace
+}  // namespace


Not needed anymore

Suggested change

namespace {

namespace {

l1slhc::L1EGCrystalCluster egcrystalcluster;

std::vector<l1slhc::L1EGCrystalCluster> l1egcrystalclustervec;

l1slhc::L1EGCrystalClusterCollection l1egcrystalclustercoll;

edm::Wrapper<l1slhc::L1EGCrystalClusterCollection> wl1egcrystalclustercoll;

L1CaloTower l1CaloTower;

std::vector<L1CaloTower> l1CaloTowervec;

L1CaloTowerCollection l1CaloTowercoll;

edm::Wrapper<L1CaloTowerCollection> wl1CaloTowercoll;

l1slhc::L1CaloJet l1calojet;

std::vector<l1slhc::L1CaloJet> l1calojetvec;

l1slhc::L1CaloJetsCollection l1calojetcoll;

edm::Wrapper<l1slhc::L1CaloJetsCollection> wl1calojetcoll;

} // namespace

} // namespace

makortel · 2020-04-02T17:49:30Z

DataFormats/L1TCalorimeterPhase2/src/classes_def.xml

+
+<lcgdict>
+
+  <class name="l1slhc::L1EGCrystalCluster" />


Should these be versioned (for future schema evolution)?

makortel · 2020-04-02T18:04:48Z

DataFormats/L1TCalorimeterPhase2/interface/L1CaloTower.h

+  int l1eg_standaloneSS = 0;
+  int l1eg_standaloneIso = 0;
+
+  bool isBarrel = false;


Please encapsulate the data properly (private members, access via member functions).

cmsbuild · 2020-04-13T19:52:56Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-04-13T19:58:18Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29348/14672

This PR adds an extra 28KB to repository
Found files with invalid states:
- DataFormats/L1TCalorimeterPhase2/interface/L1CaloJet.h:
  - Added: 45e30b9
  - Modified: 8b8eeb4, e31d796
  - Deleted: 3c6fdff
- DataFormats/L1TCalorimeterPhase2/interface/L1EGCrystalCluster.h:
  - Added: 45e30b9
  - Modified: 8b8eeb4, e31d796
  - Deleted: 3c6fdff
- DataFormats/L1TCalorimeterPhase2/interface/L1CaloTower.h:
  - Added: 45e30b9
  - Modified: 8b8eeb4, 2fe3485, e31d796, 62c0117
  - Deleted: 3c6fdff

cmsbuild · 2020-04-13T19:58:45Z

Pull request #29348 was updated. @cmsbuild, @rekovic, @benkrikler, @kpedro88 can you please check and sign again.

rekovic · 2020-04-13T20:30:17Z

please test

cmsbuild · 2020-04-13T20:30:40Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/5682/console Started: 2020/04/13 22:57

cmsbuild · 2020-04-13T22:06:29Z

+1
Tested at: 3e683bf
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5286f8/5682/summary.html
CMSSW: CMSSW_11_1_X_2020-04-13-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-04-13T22:06:33Z

Comparison job queued.

cmsbuild · 2020-04-13T23:50:32Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2695166
DQMHistoTests: Total failures: 36
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2694811
DQMHistoTests: Total skipped: 319
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

rekovic · 2020-04-14T05:35:57Z

+1

kpedro88 · 2020-04-14T15:19:39Z

+upgrade

cmsbuild · 2020-04-14T15:20:03Z

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. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2)

silviodonato · 2020-04-14T18:02:05Z

+1

rekovic added 2 commits March 31, 2020 01:30

Add DataFormats/L1TCalorimeter.

45e30b9

Apply code-check.

8b8eeb4

cmsbuild added this to the CMSSW_11_1_X milestone Mar 31, 2020

cmsbuild added code-checks-pending comparison-pending new-package-pending orp-pending pending-signatures tests-pending labels Mar 31, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 31, 2020

cmsbuild added tests-started and removed tests-pending labels Mar 31, 2020

cmsbuild added tests-rejected and removed tests-started labels Mar 31, 2020

cmsbuild added comparison-available and removed comparison-pending labels Mar 31, 2020

rekovic changed the title ~~Pr 111x L1T Phase2 DataFormats/L1TCalorimeterPhase2~~ Pr 111x New package DataFormats/L1TCalorimeterPhase2 for Phase2 L1T Apr 2, 2020

makortel reviewed Apr 2, 2020

View reviewed changes

Add missing include <vector>.

2fe3485

cmsbuild removed the code-checks-approved label Apr 2, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 13, 2020

cmsbuild added tests-started and removed tests-pending labels Apr 13, 2020

cmsbuild added tests-approved and removed tests-started labels Apr 13, 2020

cmsbuild added comparison-available and removed comparison-pending labels Apr 13, 2020

cmsbuild added l1-approved and removed l1-pending labels Apr 14, 2020

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Apr 14, 2020

cmsbuild added orp-approved and removed orp-pending labels Apr 14, 2020

cmsbuild merged commit 09b17fc into cms-sw:master Apr 14, 2020

smuzaffar mentioned this pull request Apr 15, 2020

updated lost dictionaries mapping for L1T Phase2 #29475

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New package DataFormats/L1TCalorimeterPhase2 for Phase2 L1T #29348

New package DataFormats/L1TCalorimeterPhase2 for Phase2 L1T #29348

rekovic commented Mar 31, 2020 •

edited

cmsbuild commented Mar 31, 2020

cmsbuild commented Mar 31, 2020

rekovic commented Mar 31, 2020

cmsbuild commented Mar 31, 2020 •

edited

cmsbuild commented Mar 31, 2020

rekovic commented Mar 31, 2020

cmsbuild commented Mar 31, 2020

cmsbuild commented Mar 31, 2020

cmsbuild commented Mar 31, 2020

makortel Apr 2, 2020

makortel Apr 2, 2020

makortel Apr 2, 2020

makortel Apr 2, 2020

makortel Apr 2, 2020

makortel Apr 2, 2020

makortel Apr 2, 2020

makortel Apr 2, 2020

makortel Apr 2, 2020

makortel Apr 2, 2020

cmsbuild commented Apr 13, 2020

cmsbuild commented Apr 13, 2020

cmsbuild commented Apr 13, 2020

rekovic commented Apr 13, 2020

cmsbuild commented Apr 13, 2020 •

edited

cmsbuild commented Apr 13, 2020

cmsbuild commented Apr 13, 2020

cmsbuild commented Apr 13, 2020

rekovic commented Apr 14, 2020

kpedro88 commented Apr 14, 2020

cmsbuild commented Apr 14, 2020

silviodonato commented Apr 14, 2020

	const std::map<std::string, float> GetExperimentalParams() const { return experimentalParams_; };
	const std::map<std::string, float>& experimentalParams() const { return experimentalParams_; };

	inline float GetCrystalPt(unsigned int index) const {
	inline float crystalPt(unsigned int index) const {

New package DataFormats/L1TCalorimeterPhase2 for Phase2 L1T #29348

New package DataFormats/L1TCalorimeterPhase2 for Phase2 L1T #29348

Conversation

rekovic commented Mar 31, 2020 • edited

PR description:

cmsbuild commented Mar 31, 2020

cmsbuild commented Mar 31, 2020

rekovic commented Mar 31, 2020

cmsbuild commented Mar 31, 2020 • edited

cmsbuild commented Mar 31, 2020

rekovic commented Mar 31, 2020

cmsbuild commented Mar 31, 2020

cmsbuild commented Mar 31, 2020

cmsbuild commented Mar 31, 2020

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 Apr 13, 2020

cmsbuild commented Apr 13, 2020

cmsbuild commented Apr 13, 2020

rekovic commented Apr 13, 2020

cmsbuild commented Apr 13, 2020 • edited

cmsbuild commented Apr 13, 2020

cmsbuild commented Apr 13, 2020

cmsbuild commented Apr 13, 2020

rekovic commented Apr 14, 2020

kpedro88 commented Apr 14, 2020

cmsbuild commented Apr 14, 2020

silviodonato commented Apr 14, 2020

rekovic commented Mar 31, 2020 •

edited

cmsbuild commented Mar 31, 2020 •

edited

cmsbuild commented Apr 13, 2020 •

edited