Change EGM code to be able to read HCAL PF thresholds from DB #43164

RSalvatico · 2023-11-01T17:54:38Z

PR description:

This is a PR with changes in the EGM code to allow HCAL PF cuts to be taken from DB on demand (i.e., for specific workflows) in alternative to being taken from config files (as discussed here).

Lots of adaptation work was done already on the PF side and in several aspects this should be similar.
#43025

PR validation

Several Run3 and Run2 workflows (e.g. runTheMatrix.py -l 10024.0, runTheMatrix.py -l 10824.0...)

Expected changes

None, modulo that the thresholds provided via GT are the same as previously provided via config file (which should be the case).

cmsbuild · 2023-11-01T18:03:56Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43164/37483

This PR adds an extra 64KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43164/37483/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43164/37483/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

missirol · 2023-11-02T08:03:42Z

@RSalvatico

Since "EGM code" could possibly include also code used by EGM at HLT, could you please let us know if the following plugins used at HLT should also be updated to use HcalPFCuts ? (If so, will this be done in a future PR ?)

EgammaHLTHcalVarProducerFromRecHit
FixedGridRhoProducerFastjetFromRecHit

FYI: @cms-sw/hlt-l2

swagata87 · 2023-11-03T08:22:28Z

FixedGridRhoProducerFastjetFromRecHit

I'm taking care of this.

swagata87 · 2023-11-03T09:20:17Z

@missirol #43176 here is the recHit rho code migration

RecoEgamma/EgammaPhotonProducers/src/GEDPhotonProducer.cc

smuzaffar · 2023-11-06T16:51:45Z

Milestone for this pull request has been moved to CMSSW_14_0_X.Please open a backport if it should also go in to CMSSW_13_3_X.

cmsbuild · 2023-11-07T13:25:50Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43164/37551

This PR adds an extra 32KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43164/37551/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43164/37551/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

RSalvatico · 2023-11-07T14:41:39Z

@RSalvatico

Since "EGM code" could possibly include also code used by EGM at HLT, could you please let us know if the following plugins used at HLT should also be updated to use HcalPFCuts ? (If so, will this be done in a future PR ?)
EgammaHLTHcalVarProducerFromRecHit
FixedGridRhoProducerFastjetFromRecHit
FYI: @cms-sw/hlt-l2

@missirol I am trying to take care of the other one, EgammaHLTHcalVarProducerFromRecHit, in this PR.

cmsbuild · 2023-11-27T09:54:39Z

Pull request #43164 was updated. @Martin-Grunewald, @mmusich, @jfernan2, @mandrenguyen, @cmsbuild can you please check and sign again.

mmusich · 2023-11-27T09:54:51Z

@cmsbuild, please test

cmsbuild · 2023-11-27T16:11:49Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-580732/36078/summary.html
COMMIT: aee26b2
CMSSW: CMSSW_14_0_X_2023-11-26-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43164/36078/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 76 lines from the logs
Reco comparison results: 18 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3367918
DQMHistoTests: Total failures: 1109
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3366787
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

mandrenguyen · 2023-11-27T17:38:05Z

+1

mmusich · 2023-11-28T08:16:37Z

+hlt

for RSalvatico@aee26b2
PR according to description and follow-up review;
no unexpected changes in PR tests (changes 12434.7 are spurious);

cmsbuild · 2023-11-28T08:17:06Z

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

mmusich

just some suggestions for future improvement (not for this PR!)

RecoEgamma/EgammaPhotonProducers/src/ConvertedPhotonProducer.cc

RecoEgamma/EgammaPhotonProducers/src/GEDPhotonProducer.cc

RecoEgamma/EgammaPhotonProducers/src/PhotonProducer.cc

HLTrigger/Configuration/python/HLT_75e33/modules/hltEgammaHoverEL1Seeded_cfi.py

HLTrigger/Configuration/python/HLT_75e33/modules/hltEgammaHoverEUnseeded_cfi.py

RecoEgamma/EgammaHLTProducers/plugins/EgammaHLTHcalVarProducerFromRecHit.cc

rappoccio · 2023-11-28T15:50:35Z

+1

mmusich · 2024-01-30T18:18:38Z

RecoEgamma/EgammaHLTProducers/plugins/EgammaHLTHcalVarProducerFromRecHit.cc


 class EgammaHLTHcalVarProducerFromRecHit : public edm::global::EDProducer<> {
 public:
  explicit EgammaHLTHcalVarProducerFromRecHit(const edm::ParameterSet &);

 public:
+  void beginRun(edm::Run const &, edm::EventSetup const &);


we overlooked that this module is a edm::global::EDProducer hence beginRun cannot be override-n (it's not in the base class). Hence this is never called.

The best fix would be to move the code in beginRun() to produce() and remove the hcalCuts member variable.

#43819 should fix.

cmsbuild added this to the CMSSW_13_3_X milestone Nov 1, 2023

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending code-checks-pending labels Nov 1, 2023

cmsbuild added code-checks-rejected and removed code-checks-pending labels Nov 1, 2023

swagata87 mentioned this pull request Nov 3, 2023

Migrate RecHit-based rho producer to use HCal noise cuts from GT #43176

Merged

mmusich reviewed Nov 3, 2023

View reviewed changes

RecoEgamma/EgammaPhotonProducers/src/GEDPhotonProducer.cc Outdated Show resolved Hide resolved

smuzaffar modified the milestones: CMSSW_13_3_X, CMSSW_14_0_X Nov 6, 2023

cmsbuild modified the milestones: CMSSW_14_0_X, CMSSW_13_3_X Nov 6, 2023

mmusich mentioned this pull request Nov 7, 2023

Adapt PF code to read Hcal thresholds from GT #43025

Merged

cmsbuild added code-checks-pending and removed code-checks-rejected labels Nov 7, 2023

cmsbuild added code-checks-rejected and removed code-checks-pending labels Nov 7, 2023

cmsbuild added hlt-pending and removed code-checks-rejected labels Nov 9, 2023

cmsbuild added tests-started and removed tests-pending labels Nov 27, 2023

cmsbuild added tests-approved and removed tests-started labels Nov 27, 2023

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Nov 27, 2023

cmsbuild added hlt-approved fully-signed and removed hlt-pending pending-signatures labels Nov 28, 2023

mmusich reviewed Nov 28, 2023

View reviewed changes

RecoEgamma/EgammaPhotonProducers/src/ConvertedPhotonProducer.cc Show resolved Hide resolved

RecoEgamma/EgammaPhotonProducers/src/GEDPhotonProducer.cc Show resolved Hide resolved

RecoEgamma/EgammaPhotonProducers/src/PhotonProducer.cc Show resolved Hide resolved

Martin-Grunewald reviewed Nov 28, 2023

View reviewed changes

HLTrigger/Configuration/python/HLT_75e33/modules/hltEgammaHoverEL1Seeded_cfi.py Show resolved Hide resolved

Martin-Grunewald reviewed Nov 28, 2023

View reviewed changes

HLTrigger/Configuration/python/HLT_75e33/modules/hltEgammaHoverEUnseeded_cfi.py Show resolved Hide resolved

Martin-Grunewald reviewed Nov 28, 2023

View reviewed changes

RecoEgamma/EgammaHLTProducers/plugins/EgammaHLTHcalVarProducerFromRecHit.cc Show resolved Hide resolved

cmsbuild added orp-approved and removed orp-pending labels Nov 28, 2023

cmsbuild merged commit e160d57 into cms-sw:master Nov 28, 2023
12 checks passed

This was referenced Nov 28, 2023

[LTO] Improve link time by using multiple process for linking cms-sw/cmsdist#8840

Merged

Add recipe for pytorch (C++ interface only) cms-sw/cmsdist#8388

Merged

perrotta mentioned this pull request Nov 30, 2023

Spurious comparison differences in workflows 136.X, 141.044, 4.54, ... #43456

Closed

mmusich reviewed Jan 30, 2024

View reviewed changes

mmusich mentioned this pull request Jan 31, 2024

EgammaHLTHcalVarProducerFromRecHit: fix HcalPFCuts consumes pattern #43819

Merged

RSalvatico mentioned this pull request Jan 31, 2024

Make default Phase2 HLT thresholds from config file the same as in Run3 #43822

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change EGM code to be able to read HCAL PF thresholds from DB #43164

Change EGM code to be able to read HCAL PF thresholds from DB #43164

RSalvatico commented Nov 1, 2023 •

edited

Loading

cmsbuild commented Nov 1, 2023

missirol commented Nov 2, 2023

swagata87 commented Nov 3, 2023

swagata87 commented Nov 3, 2023

smuzaffar commented Nov 6, 2023

cmsbuild commented Nov 7, 2023

RSalvatico commented Nov 7, 2023

cmsbuild commented Nov 27, 2023

mmusich commented Nov 27, 2023

cmsbuild commented Nov 27, 2023

mandrenguyen commented Nov 27, 2023

mmusich commented Nov 28, 2023

cmsbuild commented Nov 28, 2023

mmusich left a comment

rappoccio commented Nov 28, 2023

mmusich Jan 30, 2024

makortel Jan 30, 2024

mmusich Jan 31, 2024

Change EGM code to be able to read HCAL PF thresholds from DB #43164

Change EGM code to be able to read HCAL PF thresholds from DB #43164

Conversation

RSalvatico commented Nov 1, 2023 • edited Loading

PR description:

PR validation

Expected changes

cmsbuild commented Nov 1, 2023

missirol commented Nov 2, 2023

swagata87 commented Nov 3, 2023

swagata87 commented Nov 3, 2023

smuzaffar commented Nov 6, 2023

cmsbuild commented Nov 7, 2023

RSalvatico commented Nov 7, 2023

cmsbuild commented Nov 27, 2023

mmusich commented Nov 27, 2023

cmsbuild commented Nov 27, 2023

Comparison Summary

mandrenguyen commented Nov 27, 2023

mmusich commented Nov 28, 2023

cmsbuild commented Nov 28, 2023

mmusich left a comment

Choose a reason for hiding this comment

rappoccio commented Nov 28, 2023

mmusich Jan 30, 2024

Choose a reason for hiding this comment

makortel Jan 30, 2024

Choose a reason for hiding this comment

mmusich Jan 31, 2024

Choose a reason for hiding this comment

RSalvatico commented Nov 1, 2023 •

edited

Loading