Ensure cluster ecal_eta/phi are set when not running CombinedCaloLinker. #1124

EmyrClement · 2023-05-15T15:57:34Z

PR description:

This PR fixes the issue described in #1113. The cluster.ecal_eta/phi variables are set to cluster.eta/phi when we aren't running the CombinedCaloLinker. Perhaps there is a nicer way to fix this, but I believe the calo linking will ultimately be superseded by the developments in #1095.

PR validation:

A before/after comparison for jet eta/phi:

Blue/yellow line - tag before/after the bug was introduced
Green line - after applying the fix on the latest tag of the integration branch.

@gpetruc

triggerDoctor · 2023-05-16T09:21:54Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

Attempts to compile this PR failed.

The following is the stderr of the compilation attempt:

/tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc: In constructor 'L1TrackNtupleMaker::L1TrackNtupleMaker(const edm::ParameterSet&)':
/tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc:258:90: warning: 'edm::EDAnalyzer::EDAnalyzer()' is deprecated [-Wdeprecated-declarations]
  258 | L1TrackNtupleMaker::L1TrackNtupleMaker(edm::ParameterSet const& iConfig) : config(iConfig) {
      |                                                                                          ^
In file included from /tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc:11:
/cvmfs/cms.cern.ch/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_5_2_patch1/src/FWCore/Framework/interface/EDAnalyzer.h:43:20: note: declared here
   43 |     CMS_DEPRECATED EDAnalyzer();
      |                    ^~~~~~~~~~
/tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc: In member function 'virtual void L1TrackNtupleMaker::analyze(const edm::Event&, const edm::EventSetup&)':
/tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc:671:61: warning: 'bool edm::eventsetup::EventSetupRecord::get(HolderT&) const [with HolderT = edm::ESHandle<TrackerGeometry>]' is deprecated [-Wdeprecated-declarations]
  671 |   iSetup.get<TrackerDigiGeometryRecord>().get(geometryHandle);
      |                                                             ^
In file included from /cvmfs/cms.cern.ch/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_5_2_patch1/src/FWCore/Framework/interface/EventSetup.h:33,
                 from /tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc:14:
/cvmfs/cms.cern.ch/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_5_2_patch1/src/FWCore/Framework/interface/EventSetupRecord.h:110:27: note: declared here
  110 |       CMS_DEPRECATED bool get(HolderT& iHolder) const {
      |                           ^~~
/tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc:674:51: warning: 'bool edm::eventsetup::EventSetupRecord::get(HolderT&) const [with HolderT = edm::ESHandle<TrackerTopology>]' is deprecated [-Wdeprecated-declarations]
  674 |   iSetup.get<TrackerTopologyRcd>().get(tTopoHandle);
      |                                                   ^
In file included from /cvmfs/cms.cern.ch/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_5_2_patch1/src/FWCore/Framework/interface/EventSetup.h:33,
                 from /tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc:14:
/cvmfs/cms.cern.ch/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_5_2_patch1/src/FWCore/Framework/interface/EventSetupRecord.h:110:27: note: declared here
  110 |       CMS_DEPRECATED bool get(HolderT& iHolder) const {
      |                           ^~~
/tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc:677:58: warning: 'bool edm::eventsetup::EventSetupRecord::get(HolderT&) const [with HolderT = edm::ESHandle<TrackerGeometry>]' is deprecated [-Wdeprecated-declarations]
  677 |   iSetup.get<TrackerDigiGeometryRecord>().get(tGeomHandle);
      |                                                          ^
In file included from /cvmfs/cms.cern.ch/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_5_2_patch1/src/FWCore/Framework/interface/EventSetup.h:33,
                 from /tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc:14:
/cvmfs/cms.cern.ch/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_5_2_patch1/src/FWCore/Framework/interface/EventSetupRecord.h:110:27: note: declared here
  110 |       CMS_DEPRECATED bool get(HolderT& iHolder) const {
      |                           ^~~
/tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc:680:65: warning: 'bool edm::eventsetup::EventSetupRecord::get(HolderT&) const [with HolderT = edm::ESHandle<MagneticField>]' is deprecated [-Wdeprecated-declarations]
  680 |   iSetup.get<IdealMagneticFieldRecord>().get(magneticFieldHandle);
      |                                                                 ^
In file included from /cvmfs/cms.cern.ch/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_5_2_patch1/src/FWCore/Framework/interface/EventSetup.h:33,
                 from /tmp/palencia/pr_1124/new/src/L1Trigger/TrackFindingTracklet/test/L1TrackNtupleMaker.cc:14:
/cvmfs/cms.cern.ch/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_5_2_patch1/src/FWCore/Framework/interface/EventSetupRecord.h:110:27: note: declared here
  110 |       CMS_DEPRECATED bool get(HolderT& iHolder) const {
      |                           ^~~
/tmp/palencia/pr_1124/new/src/L1Trigger/L1TNtuples/plugins/L1PhaseIITreeProducer.cc: In constructor 'L1PhaseIITreeProducer::L1PhaseIITreeProducer(const edm::ParameterSet&)':
/tmp/palencia/pr_1124/new/src/L1Trigger/L1TNtuples/plugins/L1PhaseIITreeProducer.cc:189:78: warning: 'edm::EDAnalyzer::EDAnalyzer()' is deprecated [-Wdeprecated-declarations]
  189 | L1PhaseIITreeProducer::L1PhaseIITreeProducer(const edm::ParameterSet& iConfig) {
      |                                                                              ^
In file included from /tmp/palencia/pr_1124/new/src/L1Trigger/L1TNtuples/plugins/L1PhaseIITreeProducer.cc:25:
/cvmfs/cms.cern.ch/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_5_2_patch1/src/FWCore/Framework/interface/EDAnalyzer.h:43:20: note: declared here
   43 |     CMS_DEPRECATED EDAnalyzer();
      |                    ^~~~~~~~~~
/tmp/palencia/pr_1124/new/src/L1Trigger/L1TNtuples/plugins/L1PhaseIITreeProducer.cc: In member function 'virtual void L1PhaseIITreeProducer::analyze(const edm::Event&, const edm::EventSetup&)':
/tmp/palencia/pr_1124/new/src/L1Trigger/L1TNtuples/plugins/L1PhaseIITreeProducer.cc:370:9: warning: unused variable 'Z0' [-Wunused-variable]
  370 |   float Z0 = *z0Puppi;
      |         ^~
{standard input}: Assembler messages:
{standard input}:45560: Warning: end of file not at end of a line; newline inserted
c++: fatal error: Killed signal terminated program cc1plus
compilation terminated.
gmake: *** [config/SCRAM/GMake/Makefile.rules:1712: tmp/slc7_amd64_gcc10/src/L1Trigger/Phase2L1GMT/plugins/Phase2L1GMTPlugins/Phase2L1TGMTProducer.cc.o] Error 1
gmake: *** Waiting for unfinished jobs....

Info	Value
return code	`2`
command	`eval` scramv1 runtime -sh `&& scram b -j 8`

gpetruc

Ok for me

EmyrClement · 2023-05-16T09:36:39Z

@aloeliger @epalencia The trigger doctor is complaining about changes in files not touched/affected by this PR, and also in code maintained by the track finder. Do we need to fix them here or not?

triggerDoctor · 2023-05-16T09:49:29Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

Attempts to compile this PR succeeded!

Info	Value
return code	`0`
command	`eval` scramv1 runtime -sh `&& scram b -j 8`

triggerDoctor · 2023-05-16T10:02:37Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found no issues with the code checks!

Info	Value
return code	`0`
command	`eval` scramv1 runtime -sh `&& scram b -k -j 8 code-checks && scram b -k -j 8 code-checks`

I found no issues with the headers!

Info	Value
return code	`0`
command	`eval` scramv1 runtime -sh `&& scram b -k -j 8 check-headers`

triggerDoctor · 2023-05-16T10:03:04Z

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found no files with code format issues!

Info	Value
return code	`0`
command	`eval` scramv1 runtime -sh `&& scram b -k -j 8 code-format-all`

gpetruc · 2023-05-24T08:30:08Z

This one could also be ready for merge, it's merged in cmssw

aloeliger · 2023-05-24T09:18:30Z

Corresponds to cms-sw#41697

aloeliger · 2023-05-24T09:31:53Z

tagged as l1t-phase2-v64

Ensure cluster ecal_eta/phi are set when not running CombinedCaloLinker.

9097cfd

gpetruc approved these changes May 16, 2023

View reviewed changes

aloeliger added Phase-2 Pertains to phase-2 development Plot Issues labels May 16, 2023

EmyrClement mentioned this pull request May 16, 2023

Phase2 L1T - Ensure cluster ecal_eta/phi are set when not running CombinedCaloLinker. cms-sw/cmssw#41697

Merged

aloeliger merged commit 79a60e3 into cms-l1t-offline:phase2-l1t-integration-1252patch1 May 24, 2023

gpetruc mentioned this pull request May 24, 2023

Sync to cms-l1t-offline tag v64 + egamma sorter p2l1pfp/cmssw#117

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure cluster ecal_eta/phi are set when not running CombinedCaloLinker. #1124

Ensure cluster ecal_eta/phi are set when not running CombinedCaloLinker. #1124

Uh oh!

EmyrClement commented May 15, 2023

Uh oh!

triggerDoctor commented May 16, 2023

Uh oh!

gpetruc left a comment

Uh oh!

EmyrClement commented May 16, 2023

Uh oh!

triggerDoctor commented May 16, 2023

Uh oh!

triggerDoctor commented May 16, 2023

Uh oh!

triggerDoctor commented May 16, 2023

Uh oh!

gpetruc commented May 24, 2023

Uh oh!

aloeliger commented May 24, 2023

Uh oh!

aloeliger commented May 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ensure cluster ecal_eta/phi are set when not running CombinedCaloLinker. #1124

Ensure cluster ecal_eta/phi are set when not running CombinedCaloLinker. #1124

Uh oh!

Conversation

EmyrClement commented May 15, 2023

PR description:

PR validation:

Uh oh!

triggerDoctor commented May 16, 2023

Uh oh!

gpetruc left a comment

Choose a reason for hiding this comment

Uh oh!

EmyrClement commented May 16, 2023

Uh oh!

triggerDoctor commented May 16, 2023

Uh oh!

triggerDoctor commented May 16, 2023

Uh oh!

triggerDoctor commented May 16, 2023

Uh oh!

gpetruc commented May 24, 2023

Uh oh!

aloeliger commented May 24, 2023

Uh oh!

aloeliger commented May 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants