Adding HGCAL SuperCluster Regressions : 111X #32968

Sam-Harper · 2021-02-23T09:02:25Z

PR description:

This PR enables the HGCAL regressions for phase-II and is needed for the HLT TDR. It also modernises the class while doing so. RECO results will change in both phase-II and Run2/3, with the later just coming from a numerical precision effect. While there is a no change policy for RECO in non-phase-II, I would argue this is exempt as it is arguably a bug fix as it no longer needlessly casts the supercluster energy to a float and thus should go in as is

PR validation:

Tested in HLT TDR setup.

if this PR is a backport please specify the original PR and why you need to backport that PR:

backport of #32901

cmsbuild · 2021-02-23T09:02:53Z

A new Pull Request was created by @Sam-Harper (Sam Harper) for CMSSW_11_1_X.

It involves the following packages:

Configuration/AlCa
DataFormats/Common
RecoEcal/EgammaClusterAlgos
RecoEcal/EgammaClusterProducers
RecoEgamma/EgammaHLTProducers
RecoEgamma/EgammaTools

@perrotta, @smuzaffar, @Dr15Jones, @malbouis, @makortel, @slava77, @christopheralanwest, @Martin-Grunewald, @cmsbuild, @yuanchao, @tlampen, @jpata, @fwyzard, @pohsun, @francescobrivio can you please review it and eventually sign? Thanks.
@jainshilpi, @argiro, @Martin-Grunewald, @wddgit, @thomreis, @varuns23, @cericeci, @makortel, @lgray, @HuguesBrun, @simonepigazzini, @rovere, @tocheng, @mmusich, @fabiocos, @rchatter, @Fedespring, @calderona, @sobhatta, @lecriste, @afiqaize, @ram1123 this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

Backported from Adding HGCAL SuperCluster Regressions #32901

Martin-Grunewald · 2021-02-23T09:08:10Z

please test

tlampen · 2021-02-23T12:07:58Z

backport #32901

cmsbuild · 2021-02-23T13:08:34Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ecaed4/13038/summary.html
COMMIT: 9d3a65b
CMSSW: CMSSW_11_1_X_2021-02-21-0000/slc7_amd64_gcc820
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/32968/13038/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 1530 differences found in the comparisons
DQMHistoTests: Total files compared: 36
DQMHistoTests: Total histograms compared: 2784828
DQMHistoTests: Total failures: 2281
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2782497
DQMHistoTests: Total skipped: 50
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
Checked 152 log files, 31 edm output root files, 36 DQM output files

Martin-Grunewald · 2021-02-23T16:18:19Z

+1

slava77 · 2021-02-23T16:51:07Z

@silviodonato @qliphy
please clarify/confirm that 11_1_X is technically open to changes in pre-phase-2 (run2/3).
This PR has (most likely numerical precision) changes in the conversion reconstruction.

If there are some use cases for this release cycle for run2/3 production, then a somewhat safer option is to get a confirmation from relvals in 11_3_X; but it will take time and is perhaps time critical for the HLT TDR needs vs the risk of more than just numerical change in the conversions.

slava77 · 2021-02-23T17:14:57Z

@silviodonato @qliphy
please clarify/confirm that 11_1_X is technically open to changes in pre-phase-2 (run2/3).

based on the discussion in the ORP earlier today, we concluded that it's OK to proceed with changes in run2/3.

slava77 · 2021-02-24T01:54:02Z

RecoEcal/EgammaClusterProducers/src/SCEnergyCorrectorProducer.cc

+#include "FWCore/Framework/interface/MakerMacros.h"
+#include "FWCore/ParameterSet/interface/ParameterSet.h"
+
+


minor, but still, there are quite a few whitespace differences in this file compared to the variant in #32901 .
In case more updates will be needed, it would be better to port it verbatim.

slava77 · 2021-02-24T02:25:24Z

RecoEgamma/EgammaTools/interface/HGCalShowerShapeHelper.h

@@ -84,48 +85,96 @@ class HGCalShowerShapeHelper {
          sigma2ww(0.0) {}
  };

-  HGCalShowerShapeHelper(edm::ConsumesCollector &&sumes);


this file is out of sync with the master version in what looks like a missing code format.
Please update in full to avoid having someone dealing with conflicts in possible future backports.

thanks Slava, not sure how that happened

slava77 · 2021-02-24T02:27:56Z

RecoEgamma/EgammaTools/src/HGCalShowerShapeHelper.cc


-void HGCalShowerShapeHelper::initPerEvent(const edm::EventSetup &iSetup, const std::vector<reco::PFRecHit> &pfRecHits) {


this file is out of sync with the master version in what looks like a missing code format.
Please update in full to avoid having someone dealing with conflicts in possible future backports.

francescobrivio · 2021-02-24T10:26:57Z

Configuration/AlCa/python/autoCond.py

@@ -79,7 +79,7 @@
    # GlobalTag for MC production with realistic conditions for Phase1 2024
    'phase1_2024_realistic'    : '111X_mcRun3_2024_realistic_v7', # GT containing realistic conditions for Phase1 2024
    # GlobalTag for MC production with realistic conditions for Phase2
-    'phase2_realistic'         : '111X_mcRun4_realistic_T15_v4'
+    'phase2_realistic'         : '111X_mcRun4_realistic_Candidate_2021_02_11_16_39_55'


Hi @Sam-Harper, I created the versioned GT, which is equivalent to your candidate:

https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/111X_mcRun4_realistic_Candidate_2021_02_11_16_39_55/111X_mcRun4_realistic_T15_v5

Please update autoCond with the new GT.

cmsbuild · 2021-02-25T11:29:16Z

Pull request #32968 was updated. @perrotta, @smuzaffar, @Dr15Jones, @malbouis, @makortel, @slava77, @christopheralanwest, @Martin-Grunewald, @cmsbuild, @yuanchao, @tlampen, @jpata, @fwyzard, @pohsun, @francescobrivio can you please check and sign again.

Sam-Harper · 2021-02-25T11:45:11Z

Slava, thanks for the heads up about the code format. I've just done a complete rebase starting from scratch (which I had thought I had done) and it should be all good now

tlampen · 2021-02-25T12:14:24Z

please test

cmsbuild · 2021-02-25T15:50:23Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ecaed4/13083/summary.html
COMMIT: 4182450
CMSSW: CMSSW_11_1_X_2021-02-21-0000/slc7_amd64_gcc820
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/32968/13083/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 1534 differences found in the comparisons
DQMHistoTests: Total files compared: 36
DQMHistoTests: Total histograms compared: 2784828
DQMHistoTests: Total failures: 2282
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2782496
DQMHistoTests: Total skipped: 50
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
Checked 152 log files, 31 edm output root files, 36 DQM output files

slava77 · 2021-02-25T19:09:52Z

+1

for #32968 4182450

the backport is consistent with the master version, it includes the primary master Adding HGCAL SuperCluster Regressions #32901 and also ES consumes from added esConsumes to modules in RecoEcal/EgammaClusterProducers #30929
jenkins tests pass and comparisons with the baseline show differences matching what was observed in Adding HGCAL SuperCluster Regressions #32901, they affect primarily the phase-2 *FromMultiCl collections and also include some numerical precision differences in conversions.

makortel · 2021-02-25T19:13:47Z

+1

tlampen · 2021-02-25T21:59:25Z

+alca

qliphy · 2021-02-26T07:01:43Z

@cms-sw/hlt-l2 Do you have any comment?

Martin-Grunewald · 2021-02-26T10:55:57Z

+1

cmsbuild · 2021-02-26T10:56:19Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_11_1_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_11_3_X is complete. This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

qliphy · 2021-02-26T11:56:23Z

+1

cmsbuild added this to the CMSSW_11_1_X milestone Feb 23, 2021

cmsbuild added alca-pending core-pending hlt-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Feb 23, 2021

cmsbuild added tests-started and removed tests-pending labels Feb 23, 2021

cmsbuild added the backport label Feb 23, 2021

cmsbuild added tests-approved and removed tests-started labels Feb 23, 2021

cmsbuild added hlt-approved and removed hlt-pending labels Feb 23, 2021

slava77 reviewed Feb 24, 2021

View reviewed changes

cmsbuild added backport-ok and removed backport labels Feb 24, 2021

francescobrivio reviewed Feb 24, 2021

View reviewed changes

Sam-Harper added 4 commits February 25, 2021 12:01

initial support for HGCAL in regression

3d4d4e4

fixing phase2 reco config

9d6c0d9

bug fixes for HLT reg

d49fcc3

code review comments

9bc5d85

cmsbuild added hlt-pending tests-pending labels Feb 25, 2021

cmsbuild added tests-started and removed tests-pending labels Feb 25, 2021

cmsbuild added tests-approved and removed tests-started labels Feb 25, 2021

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Feb 25, 2021

cmsbuild added core-approved and removed core-pending labels Feb 25, 2021

cmsbuild added alca-approved and removed alca-pending labels Feb 25, 2021

cmsbuild added fully-signed hlt-approved and removed hlt-pending pending-signatures labels Feb 26, 2021

cmsbuild added orp-approved and removed orp-pending labels Feb 26, 2021

cmsbuild merged commit 9ceba50 into cms-sw:CMSSW_11_1_X Feb 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding HGCAL SuperCluster Regressions : 111X #32968

Adding HGCAL SuperCluster Regressions : 111X #32968

Sam-Harper commented Feb 23, 2021

cmsbuild commented Feb 23, 2021 •

edited

Martin-Grunewald commented Feb 23, 2021

tlampen commented Feb 23, 2021

cmsbuild commented Feb 23, 2021

Martin-Grunewald commented Feb 23, 2021

slava77 commented Feb 23, 2021

slava77 commented Feb 23, 2021

slava77 Feb 24, 2021

slava77 Feb 24, 2021

Sam-Harper Feb 25, 2021

slava77 Feb 24, 2021

francescobrivio Feb 24, 2021

Sam-Harper Feb 25, 2021

cmsbuild commented Feb 25, 2021

Sam-Harper commented Feb 25, 2021 •

edited

tlampen commented Feb 25, 2021

cmsbuild commented Feb 25, 2021

slava77 commented Feb 25, 2021

makortel commented Feb 25, 2021

tlampen commented Feb 25, 2021

qliphy commented Feb 26, 2021

Martin-Grunewald commented Feb 26, 2021

cmsbuild commented Feb 26, 2021

qliphy commented Feb 26, 2021

		#include "FWCore/Framework/interface/MakerMacros.h"
		#include "FWCore/ParameterSet/interface/ParameterSet.h"


		void HGCalShowerShapeHelper::initPerEvent(const edm::EventSetup &iSetup, const std::vector<reco::PFRecHit> &pfRecHits) {

Adding HGCAL SuperCluster Regressions : 111X #32968

Adding HGCAL SuperCluster Regressions : 111X #32968

Conversation

Sam-Harper commented Feb 23, 2021

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Feb 23, 2021 • edited

Martin-Grunewald commented Feb 23, 2021

tlampen commented Feb 23, 2021

cmsbuild commented Feb 23, 2021

Comparison Summary

Martin-Grunewald commented Feb 23, 2021

slava77 commented Feb 23, 2021

slava77 commented Feb 23, 2021

slava77 Feb 24, 2021

Choose a reason for hiding this comment

slava77 Feb 24, 2021

Choose a reason for hiding this comment

Sam-Harper Feb 25, 2021

Choose a reason for hiding this comment

slava77 Feb 24, 2021

Choose a reason for hiding this comment

francescobrivio Feb 24, 2021

Choose a reason for hiding this comment

Sam-Harper Feb 25, 2021

Choose a reason for hiding this comment

cmsbuild commented Feb 25, 2021

Sam-Harper commented Feb 25, 2021 • edited

tlampen commented Feb 25, 2021

cmsbuild commented Feb 25, 2021

Comparison Summary

slava77 commented Feb 25, 2021

makortel commented Feb 25, 2021

tlampen commented Feb 25, 2021

qliphy commented Feb 26, 2021

Martin-Grunewald commented Feb 26, 2021

cmsbuild commented Feb 26, 2021

qliphy commented Feb 26, 2021

cmsbuild commented Feb 23, 2021 •

edited

Sam-Harper commented Feb 25, 2021 •

edited