Merge .h with .cc files of plugins in EgammaHFProducers #32613

guitargeek · 2021-01-07T18:27:32Z

PR description:

Part of the series of PRs that merges .h files with the .cc files of the plugins in the RecoEgamma subsystem.

PR validation:

CMSSW compiles.

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

No backport intended.

cmsbuild · 2021-01-07T18:35:13Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32613/20660

This PR adds an extra 20KB to repository

cmsbuild · 2021-01-07T18:35:42Z

A new Pull Request was created by @guitargeek (Jonas Rembser) for master.

It involves the following packages:

RecoEgamma/EgammaHFProducers

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@Sam-Harper, @jainshilpi, @lgray, @sobhatta, @afiqaize, @varuns23, @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

slava77 · 2021-01-07T18:40:14Z

@cmsbuild please test

cmsbuild · 2021-01-07T21:05:14Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9988fa/12010/summary.html
COMMIT: de44d4b
CMSSW: CMSSW_11_3_X_2021-01-07-1100/slc7_amd64_gcc900

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2716967
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2716944
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
Checked 156 log files, 37 edm output root files, 37 DQM output files

perrotta · 2021-01-11T17:07:27Z

RecoEgamma/EgammaHFProducers/plugins/HFEMClusterProducer.cc

 #include "DataFormats/Common/interface/Handle.h"
-#include "FWCore/Framework/interface/ESHandle.h"


Why are you removing this?

If one includes EventSetup.h, one automatically gets the ESHandle because the EventSetup has functions that return ESHandles.

https://github.com/cms-sw/cmssw/blob/master/FWCore/Framework/interface/EventSetup.h

That means including ESHandle.h in plugin sources is generally unnecessary.

I had this idea to remove it after I saw some other ES migration PRs systematically remove includes of ESHandle.h let me see if I can find there PRs again to have a precedent to point to.

The same logic in principal also applies to Handle.h by the way.

since this file uses ESHandle, it should be included directly and not rely on something else to include it indirectly.

Okay, then I'll just change the file so it doesn't use the ESHandle anymore

perrotta · 2021-01-11T17:13:00Z

RecoEgamma/EgammaHFProducers/plugins/HFValueStruct.cc

@@ -26,7 +26,9 @@ reco::HFValueStruct::HFValueStruct(const int& version, const std::vector<double>
    doPU_ = false;
 }

-int reco::HFValueStruct::indexByIeta(int& ieta) const { return (ieta > 0) ? (abs(ieta) - 29 + 13) : (41 - abs(ieta)); }
+int reco::HFValueStruct::indexByIeta(int& ieta) const {
+  return (ieta > 0) ? (std::abs(ieta) - 29 + 13) : (41 - std::abs(ieta));


Suggested change

return (ieta > 0) ? (std::abs(ieta) - 29 + 13) : (41 - std::abs(ieta));

return (ieta > 0) ? (ieta - 29 + 13) : (41 + ieta);

Ah yes, thanks.

cmsbuild · 2021-01-12T11:04:19Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32613/20696

This PR adds an extra 20KB to repository

cmsbuild · 2021-01-12T11:04:44Z

Pull request #32613 was updated. @perrotta, @jpata, @cmsbuild, @slava77 can you please check and sign again.

perrotta · 2021-01-12T13:50:44Z

please test

perrotta · 2021-01-12T13:55:56Z

please abort

perrotta · 2021-01-12T14:01:00Z

please test

cmsbuild · 2021-01-12T14:04:25Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32613/20700

This PR adds an extra 12KB to repository

cmsbuild · 2021-01-12T14:04:46Z

Pull request #32613 was updated. @perrotta, @jpata, @slava77 can you please check and sign again.

cmsbuild · 2021-01-12T16:35:05Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9988fa/12231/summary.html
COMMIT: 32ea02a
CMSSW: CMSSW_11_3_X_2021-01-11-2300/slc7_amd64_gcc900

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2716967
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2716944
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
Checked 156 log files, 37 edm output root files, 37 DQM output files

cmsbuild · 2021-01-12T16:35:12Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9988fa/12231/summary.html
COMMIT: 32ea02a
CMSSW: CMSSW_11_3_X_2021-01-11-2300/slc7_amd64_gcc900

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2716967
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2716944
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
Checked 156 log files, 37 edm output root files, 37 DQM output files

perrotta · 2021-01-12T16:39:54Z

+1

Technical, it does what anticipated in the title
Jenkisn tests pass

cmsbuild · 2021-01-12T16:40:16Z

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

qliphy · 2021-01-13T00:11:00Z

+1

cmsbuild added this to the CMSSW_11_3_X milestone Jan 7, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jan 7, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 7, 2021

cmsbuild added tests-started and removed tests-pending labels Jan 7, 2021

cmsbuild added tests-approved and removed tests-started labels Jan 7, 2021

perrotta reviewed Jan 11, 2021

View reviewed changes

guitargeek added 2 commits January 12, 2021 11:51

merge .h with .cc files of plugins in EgammaHFProducers

e6eeb99

implement review comments

de867bd

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-approved labels Jan 12, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 12, 2021

cmsbuild added tests-started and removed tests-pending labels Jan 12, 2021

cmsbuild added tests-pending and removed tests-started labels Jan 12, 2021

fix sign of ieta

32ea02a

cmsbuild added code-checks-pending and removed code-checks-approved labels Jan 12, 2021

cmsbuild added tests-started and removed tests-pending labels Jan 12, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 12, 2021

cmsbuild added tests-approved and removed tests-started labels Jan 12, 2021

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Jan 12, 2021

guitargeek mentioned this pull request Jan 12, 2021

Use EDPutToken in ReducedEGProducer and modernize GEDPhotonCoreProducer #32626

Merged

cmsbuild added orp-approved and removed orp-pending labels Jan 13, 2021

cmsbuild merged commit e5ec382 into cms-sw:master Jan 13, 2021

guitargeek deleted the EgammaHFProducers_1 branch January 13, 2021 11:40

guitargeek mentioned this pull request Jun 1, 2021

Merge .h with .cc files in RecoEcal/EgammaClusterProducers #33935

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge .h with .cc files of plugins in EgammaHFProducers #32613

Merge .h with .cc files of plugins in EgammaHFProducers #32613

guitargeek commented Jan 7, 2021

cmsbuild commented Jan 7, 2021

cmsbuild commented Jan 7, 2021

slava77 commented Jan 7, 2021

cmsbuild commented Jan 7, 2021

perrotta Jan 11, 2021

guitargeek Jan 12, 2021

davidlange6 Jan 12, 2021

guitargeek Jan 12, 2021

perrotta Jan 11, 2021

guitargeek Jan 12, 2021

cmsbuild commented Jan 12, 2021

cmsbuild commented Jan 12, 2021

perrotta commented Jan 12, 2021

perrotta commented Jan 12, 2021

perrotta commented Jan 12, 2021

cmsbuild commented Jan 12, 2021

cmsbuild commented Jan 12, 2021

cmsbuild commented Jan 12, 2021

cmsbuild commented Jan 12, 2021

perrotta commented Jan 12, 2021

cmsbuild commented Jan 12, 2021

qliphy commented Jan 13, 2021

		#include "DataFormats/Common/interface/Handle.h"
		#include "FWCore/Framework/interface/ESHandle.h"

	return (ieta > 0) ? (std::abs(ieta) - 29 + 13) : (41 - std::abs(ieta));
	return (ieta > 0) ? (ieta - 29 + 13) : (41 + ieta);

Merge .h with .cc files of plugins in EgammaHFProducers #32613

Merge .h with .cc files of plugins in EgammaHFProducers #32613

Conversation

guitargeek commented Jan 7, 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 Jan 7, 2021

cmsbuild commented Jan 7, 2021

slava77 commented Jan 7, 2021

cmsbuild commented Jan 7, 2021

Comparison Summary

perrotta Jan 11, 2021

Choose a reason for hiding this comment

guitargeek Jan 12, 2021

Choose a reason for hiding this comment

davidlange6 Jan 12, 2021

Choose a reason for hiding this comment

guitargeek Jan 12, 2021

Choose a reason for hiding this comment

perrotta Jan 11, 2021

Choose a reason for hiding this comment

guitargeek Jan 12, 2021

Choose a reason for hiding this comment

cmsbuild commented Jan 12, 2021

cmsbuild commented Jan 12, 2021

perrotta commented Jan 12, 2021

perrotta commented Jan 12, 2021

perrotta commented Jan 12, 2021

cmsbuild commented Jan 12, 2021

cmsbuild commented Jan 12, 2021

cmsbuild commented Jan 12, 2021

Comparison Summary

cmsbuild commented Jan 12, 2021

Comparison Summary

perrotta commented Jan 12, 2021

cmsbuild commented Jan 12, 2021

qliphy commented Jan 13, 2021