Removing GEDGsfElectronProducer - No inheriting electron producers anymore #29303

guitargeek · 2020-03-25T16:50:17Z

PR description:

After a few previous PRs in the EgammaElectronProducers package, this is the PR that removes the last electron producer that derives from the GsfElectronBaseProducer class.

In in past, there were 4 producers which derived from GsfElectronBaseProducer (check for example 10_2_X):

GEDGsfElectronProducer
GsfElectronProducer
GsfElectronEcalDrivenProducer
LowPtGsfElectronProducer

Number 3 and 4 were so similar to the base class that their functionality got moved into the base class so they could be removed. Number 2 was not used since Run 1 and got removed in #28429. Number 1, the GEDGsfElectronProducer, undergoes a similar treatment than 3 and 4 in this PR: since it extends very little on the base class, the functionality just got moved into free functions that will be used by the base producer if a configuration flag is set. Furthermore, the old GEDGsfElectronProducer produced an additional ValueMap product, and this PR factorizes this out into a new GEDGsfElectronValueMapProducer, improving the modularity of the reconstruction.

Now, inheritance is finally not used anymore as a method for configuration in the EgammaElectronProducers plugins, which makes the code easier to understand. That also allows to merge the .h and .cc files of the former base producer.

Edit: during the review it was decided to rename the surviving GsfElectronBaseProducer producer to GsfElectronProducer because it is not intended to be used as a base class anymore.

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 · 2020-03-25T16:50:43Z

The code-checks are being triggered in jenkins.

guitargeek · 2020-03-25T16:51:58Z

Forgot to squash the commits

cmsbuild · 2020-03-25T16:52:07Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-03-25T16:56:00Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29303/14376

This PR adds an extra 40KB to repository

cmsbuild · 2020-03-25T16:56:24Z

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

It involves the following packages:

Calibration/EcalAlCaRecoProducers
RecoEgamma/EgammaElectronProducers
RecoEgamma/EgammaIsolationAlgos
RecoParticleFlow/PFProducer

@perrotta, @slava77, @christopheralanwest, @tocheng, @cmsbuild, @tlampen, @pohsun can you please review it and eventually sign? Thanks.
@mmarionncern, @Sam-Harper, @jainshilpi, @afiqaize, @varuns23, @tocheng, @lgray, @sobhatta, @rovere, @lecriste, @hatakeyamak, @mmusich, @argiro, @bachtis, @seemasharmafnal, @cbernet this is something you requested to watch as well.
@davidlange6, @silviodonato, @fabiocos you are the release manager for this.

cms-bot commands are listed here

slava77 · 2020-03-25T17:19:07Z

@cmsbuild please test

cmsbuild · 2020-03-25T17:19:31Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/5385/console Started: 2020/03/25 18:23

cmsbuild · 2020-03-25T18:37:39Z

+1
Tested at: d37d345
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f63ffc/5385/summary.html
CMSSW: CMSSW_11_1_X_2020-03-25-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-03-25T18:37:41Z

Comparison job queued.

cmsbuild · 2020-03-25T20:06:47Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2692110
DQMHistoTests: Total failures: 38
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2691753
DQMHistoTests: Total skipped: 319
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

pohsun · 2020-04-07T03:32:20Z

+1

cmsbuild · 2020-04-08T20:23:04Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/5611/console Started: 2020/04/08 22:28

cmsbuild · 2020-04-08T20:23:05Z

Pull request #29303 was updated. @perrotta, @slava77, @christopheralanwest, @tocheng, @cmsbuild, @tlampen, @pohsun can you please check and sign again.

cmsbuild · 2020-04-08T21:40:06Z

+1
Tested at: 6205e84
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f63ffc/5611/summary.html
CMSSW: CMSSW_11_1_X_2020-04-08-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-04-08T21:40:08Z

Comparison job queued.

cmsbuild · 2020-04-08T23:08:21Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2692110
DQMHistoTests: Total failures: 33
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2691758
DQMHistoTests: Total skipped: 319
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

slava77 · 2020-04-09T22:27:48Z

+1

for #29303 6205e84

code changes (refactoring) are in line with the PR description (mostly; a clarification from the last update can be adde) and with the follow up review.
jenkins tests pass and comparisons with the baseline show no (relevant) differences

silviodonato · 2020-04-10T09:38:07Z

do you have comments @pohsun @tlampen @tocheng @christopheralanwest?

guitargeek · 2020-04-10T11:49:19Z

I updated the description according to the review.

slava77 · 2020-04-14T15:16:20Z

do you have comments @pohsun @tlampen @tocheng @christopheralanwest?

ping

guitargeek · 2020-04-16T19:39:26Z

Hi @silviodonato, is it possible to merge this? The changes in alca are just one line which got covered by the tests. We know that for sure because I made a mistake in exactly that line earlier and the tests failed at that point. I got asked some questions about the GsfElectronProducer code, and it would be nice if I could already hyperlink to the newest version of the source in master.

tocheng · 2020-04-16T19:45:20Z

+1

cmsbuild · 2020-04-16T19:45:48Z

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

silviodonato · 2020-04-17T06:36:10Z

+1

cmsbuild added this to the CMSSW_11_1_X milestone Mar 25, 2020

cmsbuild added alca-pending code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Mar 25, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 25, 2020

cmsbuild added tests-started and removed tests-pending labels Mar 25, 2020

cmsbuild added tests-approved and removed tests-started labels Mar 25, 2020

guitargeek mentioned this pull request Mar 25, 2020

Remove unused SeedFilter class in EgammaElectronAlgos to simplify ElectronSeedProducer #29304

Merged

cmsbuild added comparison-available and removed comparison-pending labels Mar 25, 2020

cmsbuild added alca-approved and removed alca-pending labels Apr 7, 2020

cmsbuild added tests-started and removed tests-pending labels Apr 8, 2020

cmsbuild added tests-approved and removed tests-started labels Apr 8, 2020

cmsbuild added comparison-available and removed comparison-pending labels Apr 8, 2020

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Apr 9, 2020

cmsbuild mentioned this pull request Apr 16, 2020

From cmssw 10 2 21 #29490

Closed

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels Apr 16, 2020

cmsbuild added orp-approved and removed orp-pending labels Apr 17, 2020

cmsbuild merged commit d252e7b into cms-sw:master Apr 17, 2020

guitargeek deleted the GEDGsfElectronValueMapProducer_1 branch April 17, 2020 08:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removing GEDGsfElectronProducer - No inheriting electron producers anymore #29303

Removing GEDGsfElectronProducer - No inheriting electron producers anymore #29303

guitargeek commented Mar 25, 2020 •

edited

cmsbuild commented Mar 25, 2020

guitargeek commented Mar 25, 2020

cmsbuild commented Mar 25, 2020

cmsbuild commented Mar 25, 2020

cmsbuild commented Mar 25, 2020

slava77 commented Mar 25, 2020

cmsbuild commented Mar 25, 2020 •

edited

cmsbuild commented Mar 25, 2020

cmsbuild commented Mar 25, 2020

cmsbuild commented Mar 25, 2020

pohsun commented Apr 7, 2020

cmsbuild commented Apr 8, 2020 •

edited

cmsbuild commented Apr 8, 2020

cmsbuild commented Apr 8, 2020

cmsbuild commented Apr 8, 2020

cmsbuild commented Apr 8, 2020

slava77 commented Apr 9, 2020

silviodonato commented Apr 10, 2020

guitargeek commented Apr 10, 2020

slava77 commented Apr 14, 2020

guitargeek commented Apr 16, 2020

tocheng commented Apr 16, 2020

cmsbuild commented Apr 16, 2020

silviodonato commented Apr 17, 2020

Removing GEDGsfElectronProducer - No inheriting electron producers anymore #29303

Removing GEDGsfElectronProducer - No inheriting electron producers anymore #29303

Conversation

guitargeek commented Mar 25, 2020 • edited

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 Mar 25, 2020

guitargeek commented Mar 25, 2020

cmsbuild commented Mar 25, 2020

cmsbuild commented Mar 25, 2020

cmsbuild commented Mar 25, 2020

slava77 commented Mar 25, 2020

cmsbuild commented Mar 25, 2020 • edited

cmsbuild commented Mar 25, 2020

cmsbuild commented Mar 25, 2020

cmsbuild commented Mar 25, 2020

pohsun commented Apr 7, 2020

cmsbuild commented Apr 8, 2020 • edited

cmsbuild commented Apr 8, 2020

cmsbuild commented Apr 8, 2020

cmsbuild commented Apr 8, 2020

cmsbuild commented Apr 8, 2020

slava77 commented Apr 9, 2020

silviodonato commented Apr 10, 2020

guitargeek commented Apr 10, 2020

slava77 commented Apr 14, 2020

guitargeek commented Apr 16, 2020

tocheng commented Apr 16, 2020

cmsbuild commented Apr 16, 2020

silviodonato commented Apr 17, 2020

guitargeek commented Mar 25, 2020 •

edited

cmsbuild commented Mar 25, 2020 •

edited

cmsbuild commented Apr 8, 2020 •

edited