[EGM HLT] Produce sigma_IPhi_IPhi, needed for regression #35486

swagata87 · 2021-09-30T12:38:03Z

PR description:

In EGM HLT producer for cluster shape variables, adding sigmaIPhiIPhi, which is needed in HLT supercluster regression setup.

PR validation:

Ran EGM HLT on DY sample, after merging this branch and checked sigmaIPhiIPhi distributions.

This PR is not a backport.
Backport is not needed.

cmsbuild · 2021-09-30T12:45:17Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35486/25655

This PR adds an extra 16KB 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-35486/25655/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35486/25655/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-09-30T13:01:45Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35486/25656

This PR adds an extra 16KB to repository

cmsbuild · 2021-09-30T13:02:08Z

A new Pull Request was created by @swagata87 (Swagata Mukherjee) for master.

It involves the following packages:

RecoEgamma/EgammaHLTProducers (hlt)

@Martin-Grunewald, @missirol, @cmsbuild can you please review it and eventually sign? Thanks.
@Sam-Harper, @HuguesBrun, @silviodonato, @jainshilpi, @Fedespring, @lgray, @calderona, @sscruz, @afiqaize, @wrtabb, @ram1123, @varuns23, @cericeci, @sobhatta this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

Martin-Grunewald · 2021-09-30T13:13:22Z

please test

missirol

@swagata87, could you please remind me if this just needed for ongoing studies, or is part of updates already discussed in TSG?

missirol · 2021-09-30T13:31:04Z

RecoEgamma/EgammaHLTProducers/plugins/EgammaHLTClusterShapeProducer.cc

  for (unsigned int iRecoEcalCand = 0; iRecoEcalCand < recoecalcandHandle->size(); iRecoEcalCand++) {
    reco::RecoEcalCandidateRef recoecalcandref(recoecalcandHandle, iRecoEcalCand);
    if (recoecalcandref->superCluster()->seed()->seed().det() != DetId::Ecal) {  //HGCAL, skip for now
      clshMap.insert(recoecalcandref, 0);
      clsh5x5Map.insert(recoecalcandref, 0);
+      clsh5x5NoiseCleanedMap.insert(recoecalcandref, 0);


Just for my own understanding. This looks like a fix to fill one of the existing maps for candidates in HGCal, but I assume it has no impact on previous EGM results for Phase-2. Correct? And if so, why?

yes this has no impact on Phase2 HLT TDR results of EGM because NoiseCleaned shower-shapes entered CMSSW only in 11_3_X via this PR #33148. The HLT TDR release, 11_1_X, do not have noise-cleaned e/gamma variables. For phase2 studies in barrel until now, we have been using the usual Full5x5 showershapes, where no noise-cleaning is applied.

missirol · 2021-09-30T13:39:34Z

RecoEgamma/EgammaHLTProducers/plugins/EgammaHLTClusterShapeProducer.cc

+                                                                       &thresholds,
+                                                                       multThresEB_,
+                                                                       multThresEE_)[2]);
+


Looks like the 4 calls to lazyTools5x5.localCovariances could be reduced at least to 2. No? Something like

auto const ecalCandLocalCov = lazyTools5x5.localCovariances(*(recoecalcandref->superCluster()->seed())); auto const sigmaee5x5 = sqrt(ecalCandLocalCov[0]); auto const sigmapp5x5 = sqrt(ecalCandLocalCov[2]); auto const ecalCandLocalCovNoiseCleaned = lazyTools5x5.localCovariances(*(recoecalcandref->superCluster()->seed()), EgammaLocalCovParamDefaults::kRelEnCut, &thresholds, multThresEB_, multThresEE_); auto const sigmaee5x5NoiseCleaned = sqrt(ecalCandLocalCovNoiseCleaned[0]); auto const sigmapp5x5NoiseCleaned = sqrt(ecalCandLocalCovNoiseCleaned[2]);

cmsbuild · 2021-09-30T16:19:56Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-908bab/19284/summary.html
COMMIT: 7362497
CMSSW: CMSSW_12_1_X_2021-09-30-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35486/19284/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: 10 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 3211080
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3211052
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

swagata87 · 2021-09-30T18:51:43Z

could you please remind me if this just needed for ongoing studies, or is part of updates already discussed in TSG?

so this PR is not extremely urgent in the sense that this variable sigmaIPhiIPhi is not used in any HLT filter that we have in egamma, but it is needed only as a pre-selection cut variable for training regression. We can pretty well run our regression setup by merging this branch by hand, but it's kinda annoying and its easier if this is merged in cmssw.

EGM HLT regression was discussed in TSG, but I don't think the need of this PR was explicitly discussed.

cmsbuild · 2021-09-30T20:15:49Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35486/25671

This PR adds an extra 16KB 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-35486/25671/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35486/25671/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-09-30T20:23:43Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35486/25674

This PR adds an extra 20KB to repository

cmsbuild · 2021-09-30T20:24:04Z

Pull request #35486 was updated. @Martin-Grunewald, @missirol, @cmsbuild can you please check and sign again.

missirol · 2021-09-30T21:31:50Z

@cmsbuild, please test

missirol · 2021-09-30T21:38:34Z

@cmsbuild , please test

cmsbuild · 2021-10-01T01:05:26Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-908bab/19308/summary.html
COMMIT: 2852442
CMSSW: CMSSW_12_1_X_2021-09-30-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35486/19308/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

No significant changes to the logs found
Reco comparison results: 1000 differences found in the comparisons
DQMHistoTests: Total files compared: 40
DQMHistoTests: Total histograms compared: 3211080
DQMHistoTests: Total failures: 2568
DQMHistoTests: Total nulls: 62
DQMHistoTests: Total successes: 3208428
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 65.355 KiB( 39 files compared)
DQMHistoSizes: changed ( 140.53 ): 63.680 KiB Hcal/DigiRunHarvesting
DQMHistoSizes: changed ( 140.53 ): 1.676 KiB RPC/DCSInfo
Checked 169 log files, 37 edm output root files, 40 DQM output files
TriggerResults: no differences found

missirol · 2021-10-01T06:04:16Z

+hlt

cmsbuild · 2021-10-01T06:04:40Z

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

perrotta · 2021-10-01T08:42:14Z

+1

swagata87 added 2 commits September 30, 2021 09:44

adding sigmaIPhiIPhi

bc395bf

adding some comments for clarity

4bd7eaf

cmsbuild added this to the CMSSW_12_1_X milestone Sep 30, 2021

cmsbuild added code-checks-pending hlt-pending orp-pending pending-signatures tests-pending labels Sep 30, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Sep 30, 2021

code checks

7362497

cmsbuild added code-checks-pending and removed code-checks-rejected labels Sep 30, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 30, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 30, 2021

missirol reviewed Sep 30, 2021

View reviewed changes

cmsbuild added tests-approved and removed tests-started labels Sep 30, 2021

lesser calls to lazytool

915eaf2

cmsbuild added code-checks-pending and removed tests-approved code-checks-approved labels Sep 30, 2021

cmsbuild added the tests-pending label Sep 30, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Sep 30, 2021

code checks

2852442

cmsbuild added code-checks-pending and removed code-checks-rejected labels Sep 30, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 30, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 30, 2021

cmsbuild added tests-approved and removed tests-started labels Oct 1, 2021

cmsbuild added fully-signed hlt-approved and removed hlt-pending pending-signatures labels Oct 1, 2021

cmsbuild added orp-approved and removed orp-pending labels Oct 1, 2021

cmsbuild merged commit 5e0e54f into cms-sw:master Oct 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EGM HLT] Produce sigma_IPhi_IPhi, needed for regression #35486

[EGM HLT] Produce sigma_IPhi_IPhi, needed for regression #35486

swagata87 commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

Martin-Grunewald commented Sep 30, 2021

missirol left a comment

missirol Sep 30, 2021

swagata87 Sep 30, 2021

missirol Sep 30, 2021

cmsbuild commented Sep 30, 2021

swagata87 commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

missirol commented Sep 30, 2021

missirol commented Sep 30, 2021

cmsbuild commented Oct 1, 2021

missirol commented Oct 1, 2021

cmsbuild commented Oct 1, 2021

perrotta commented Oct 1, 2021

[EGM HLT] Produce sigma_IPhi_IPhi, needed for regression #35486

[EGM HLT] Produce sigma_IPhi_IPhi, needed for regression #35486

Conversation

swagata87 commented Sep 30, 2021

PR description:

PR validation:

cmsbuild commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

Martin-Grunewald commented Sep 30, 2021

missirol left a comment

Choose a reason for hiding this comment

missirol Sep 30, 2021

Choose a reason for hiding this comment

swagata87 Sep 30, 2021

Choose a reason for hiding this comment

missirol Sep 30, 2021

Choose a reason for hiding this comment

cmsbuild commented Sep 30, 2021

Comparison Summary

swagata87 commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

cmsbuild commented Sep 30, 2021

missirol commented Sep 30, 2021

missirol commented Sep 30, 2021

cmsbuild commented Oct 1, 2021

Comparison Summary

missirol commented Oct 1, 2021

cmsbuild commented Oct 1, 2021

perrotta commented Oct 1, 2021