Move eta phi region from reco egamma to math

[perrotta]

In a discussion started in https://hypernews.cern.ch/HyperNews/CMS/get/recoDevelopment/1576.html @fwyzard noticed that there was an unwanted remaining dependence of Geometry on Reco.

The RecoEcal/EgammaCoreTools/interface/EcalEtaPhiRegion.h was not specific for reco, and not even strictly to ECAL, as it just provides a method to verify whether a given (eta,phi) direction is inside a given rectangular region in the eta,phi plane.

The idea was therefore to move the class into DataFormats/Math, where there are already other similar classes, and rename it more generically and without any reference to ECAL. The name "EtaPhiRegion" was already taken by a struct defined in RecoEgamma/EgammaHLTProducers/interface/HLTCaloObjInRegionsProducer.h. Therefore, in order to avoid possible duplicates, I opted to "RectangularEtaPhiRegion".

Comments about the name and the position of the moved package are welcome.

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22593/3926

[cmsbuild]

A new Pull Request was created by @perrotta for master.

It involves the following packages:

Calibration/HcalIsolatedTrackReco
DQMOffline/CalibCalo
DQMOffline/EGamma
DataFormats/Math
Geometry/EcalMapping
RecoEcal/EgammaClusterAlgos
RecoEcal/EgammaClusterProducers
RecoEcal/EgammaCoreTools
RecoEgamma/EgammaHLTProducers

@perrotta, @ghellwig, @civanch, @Dr15Jones, @vazzolini, @kmaeshima, @arunhep, @ianna, @mdhildreth, @dmitrijus, @Martin-Grunewald, @silviodonato, @franzoni, @jfernan2, @cmsbuild, @cerminar, @slava77, @fwyzard, @vanbesien, @lpernie can you please review it and eventually sign? Thanks.
@ghellwig, @Sam-Harper, @battibass, @makortel, @felicepantaleo, @jainshilpi, @varuns23, @tocheng, @lgray, @calderona, @HuguesBrun, @rovere, @rociovilar, @mmusich, @argiro, @ebrondol, @folguera, @drkovalskyi this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[perrotta]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/26811/console Started: 2018/03/13 12:38

[dmitrijus]

+1

[cmsbuild]

+1
Tested at: 5faf690
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22593/26811/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22593/26811/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 29
• DQMHistoTests: Total histograms compared: 2477528
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2477351
• DQMHistoTests: Total skipped: 176
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.959999999977 KiB( 22 files compared)
• Checked 118 log files, 9 edm output root files, 29 DQM output files

[ianna]

+1

[Martin-Grunewald]

+1

[arunhep]

+1

[perrotta]

@argiro : do you see any possible issue for ECAL DPG because of this PR?

[perrotta]

Let me also add EGamma contacts here: @Sam-Harper @jainshilpi @varuns23
If nobody complains, by next week I will sign this for reco

[perrotta]

+1

• (No complains arrived from ECal or EGamma)
• Remove unwanted remaining dependence of Geometry on Reco by moving EcalEtaPhiRegion from RecoEcal/EgammaCoreTools to DataFormats/Math
  • Since the method is general enough, the reference to "Ecal" is dropped from its name
  • All dependencies in CMSSW are updated accordingly

[cmsbuild]

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

[fabiocos]

+1