ElectronNHitSeedProducer modernization #29512

guitargeek · 2020-04-17T21:22:46Z

PR description:

In #29304 it was discussed that the ElectronSeedProducer should be replaced at some point with the ElectronNHitSeedProducer which is already used at HLT. That's indeed a good idea, but it would be good if we update the ElectronNHitSeedProducer first to fulfil some good practices for producer plugins, which this PR implements:

Usage of EDPutToken
Usage of ESGetToken instead of caching the event setup objects manually (the framework does this efficiently for us)
Move from stream to global module to enforce good design practice (like const functions and immutable algorithm classes)

In particular, the move to a global module was achieved by splitting the TrajSeedMatcher class up in a const configuration structure which is initialized in the constructor of the plugin, and an actual algorithm class which is instantiated for each event using a reference to the configuration. This pattern is already used in some other plugins (like the GsfElectronAlgo) and I think it makes sense.

PR validation:

CMSSW compiles and local matrix tests pass.

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

No backport intended.

cmsbuild · 2020-04-17T21:24:22Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-04-17T21:30:42Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29512/14762

This PR adds an extra 1072KB to repository

cmsbuild · 2020-04-17T21:31:07Z

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

It involves the following packages:

HLTrigger/Configuration
RecoEgamma/EgammaElectronAlgos
RecoEgamma/EgammaElectronProducers

@perrotta, @cmsbuild, @fwyzard, @Martin-Grunewald, @slava77 can you please review it and eventually sign? Thanks.
@Sam-Harper, @makortel, @jainshilpi, @rovere, @lgray, @Martin-Grunewald, @sobhatta, @lecriste, @afiqaize, @varuns23 this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

slava77 · 2020-04-17T21:37:45Z

@cmsbuild please test

cmsbuild · 2020-04-17T21:38:05Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/5757/console Started: 2020/04/17 23:44

cmsbuild · 2020-04-17T22:52:06Z

-1

Tested at: 8c71f7d

CMSSW: CMSSW_11_1_X_2020-04-17-1100
SCRAM_ARCH: slc7_amd64_gcc820
You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fd74d6/5757/summary.html

I found follow errors while testing this PR

Failed tests: RelVals AddOn

RelVals:

When I ran the RelVals I found an error in the following workflows:
11634.0 step2

runTheMatrix-results/11634.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2021_GenSimFull+DigiFull_2021+RecoFull_2021+HARVESTFull_2021+ALCAFull_2021/step2_TTbar_14TeV+TTbar_14TeV_TuneCP5_2021_GenSimFull+DigiFull_2021+RecoFull_2021+HARVESTFull_2021+ALCAFull_2021.log

12434.0 step2

runTheMatrix-results/12434.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2023_GenSimFull+DigiFull_2023+RecoFull_2023+HARVESTFull_2023+ALCAFull_2023/step2_TTbar_14TeV+TTbar_14TeV_TuneCP5_2023_GenSimFull+DigiFull_2023+RecoFull_2023+HARVESTFull_2023+ALCAFull_2023.log

AddOn:

I found errors in the following addon tests:

cmsRun /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_1_X_2020-04-17-1100/src/HLTrigger/Configuration/test/OnLine_HLT_2018.py realData=False globalTag=@ inputFiles=@ : FAILED - time: date Sat Apr 18 00:46:06 2020-date Sat Apr 18 00:33:29 2020 s - exit: 16640
cmsDriver.py RelVal -s HLT:2018,RAW2DIGI,L1Reco,RECO --mc --scenario=pp -n 10 --conditions auto:run2_mc_2018 --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2018 --processName=HLTRECO --filein file:RelVal_Raw_2018_MC.root --fileout file:RelVal_Raw_2018_MC_HLT_RECO.root : FAILED - time: date Sat Apr 18 00:46:06 2020-date Sat Apr 18 00:33:29 2020 s - exit: 16640
cmsRun /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_1_X_2020-04-17-1100/src/HLTrigger/Configuration/test/OnLine_HLT_GRun.py realData=False globalTag=@ inputFiles=@ : FAILED - time: date Sat Apr 18 00:45:54 2020-date Sat Apr 18 00:33:38 2020 s - exit: 16640
cmsDriver.py RelVal -s HLT:GRun,RAW2DIGI,L1Reco,RECO --mc --scenario=pp -n 10 --conditions auto:run3_mc_GRun --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run3 --processName=HLTRECO --filein file:RelVal_Raw_GRun_MC.root --fileout file:RelVal_Raw_GRun_MC_HLT_RECO.root : FAILED - time: date Sat Apr 18 00:45:54 2020-date Sat Apr 18 00:33:38 2020 s - exit: 16640
cmsRun /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_1_X_2020-04-17-1100/src/HLTrigger/Configuration/test/OnLine_HLT_2018.py realData=True globalTag=@ inputFiles=@ : FAILED - time: date Sat Apr 18 00:44:29 2020-date Sat Apr 18 00:33:43 2020 s - exit: 16640
cmsDriver.py RelVal -s HLT:2018,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run2_data_2018 --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2018 --processName=HLTRECO --filein file:RelVal_Raw_2018_DATA.root --fileout file:RelVal_Raw_2018_DATA_HLT_RECO.root : FAILED - time: date Sat Apr 18 00:44:29 2020-date Sat Apr 18 00:33:43 2020 s - exit: 16640
cmsRun /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_1_X_2020-04-17-1100/src/HLTrigger/Configuration/test/OnLine_HLT_HIon.py realData=False globalTag=@ inputFiles=@ : FAILED - time: date Sat Apr 18 00:42:38 2020-date Sat Apr 18 00:33:58 2020 s - exit: 16640
cmsDriver.py RelVal -s HLT:HIon,RAW2DIGI,L1Reco,RECO --mc --scenario=pp -n 10 --conditions auto:run3_mc_HIon --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run3_pp_on_PbPb --processName=HLTRECO --filein file:RelVal_Raw_HIon_MC.root --fileout file:RelVal_Raw_HIon_MC_HLT_RECO.root : FAILED - time: date Sat Apr 18 00:42:38 2020-date Sat Apr 18 00:33:58 2020 s - exit: 16640
cmsRun /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_1_X_2020-04-17-1100/src/HLTrigger/Configuration/test/OnLine_HLT_GRun.py realData=True globalTag=@ inputFiles=@ : FAILED - time: date Sat Apr 18 00:47:21 2020-date Sat Apr 18 00:37:49 2020 s - exit: 16640
cmsDriver.py RelVal -s HLT:GRun,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run3_data_GRun --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run3 --processName=HLTRECO --filein file:RelVal_Raw_GRun_DATA.root --fileout file:RelVal_Raw_GRun_DATA_HLT_RECO.root : FAILED - time: date Sat Apr 18 00:47:21 2020-date Sat Apr 18 00:37:49 2020 s - exit: 16640

cmsbuild · 2020-04-17T22:52:09Z

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

Martin-Grunewald · 2020-04-18T06:19:52Z

-1
No direct modifications of HLT menu files, as they are generated dumps.
Instead, put the customisation all plugin instances into HLTrigger/Configuration/python/customizeHLTforCMSSW.py

guitargeek · 2020-04-18T08:16:51Z

Thanks for the explanations and instructions, I was just about to ask!

cmsbuild · 2020-04-27T17:24:00Z

+1
Tested at: d28cda9
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fd74d6/5869/summary.html
CMSSW: CMSSW_11_1_X_2020-04-27-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-04-27T17:24:03Z

Comparison job queued.

cmsbuild · 2020-04-27T18:53:34Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2696435
DQMHistoTests: Total failures: 25
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2696091
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

perrotta · 2020-05-04T13:07:18Z

+1

Modernization, simplification and switch to being a global module of the ElectronNHitSeedProducer is implemented according to what specified in the PR description, including the bug fix cited in ElectronNHitSeedProducer modernization #29512 (comment) (ininfluent in all current workflows and HLT menus because the enableHitSkipping steering parameter is never modified from the default false value as of now)
Jenkins tests pass and show no difference in the reco or in the DQM outputs (DQM would account for the possible differences in the HLT outputs)

Martin-Grunewald · 2020-05-05T07:26:16Z

+1

cmsbuild · 2020-05-05T07:26:46Z

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-05-05T08:15:57Z

+1

cmsbuild added this to the CMSSW_11_1_X milestone Apr 17, 2020

cmsbuild added code-checks-pending comparison-pending hlt-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 17, 2020

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

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

cmsbuild added comparison-notrun tests-rejected and removed comparison-pending tests-started labels Apr 17, 2020

cmsbuild added hlt-rejected and removed hlt-pending labels Apr 18, 2020

cmsbuild removed code-checks-approved comparison-notrun hlt-rejected labels Apr 21, 2020

cmsbuild added the tests-started label Apr 27, 2020

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

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

cmsbuild mentioned this pull request May 4, 2020

[HBHE] Always use 8 pulses in MAHI without 1-pulse to 8-pulse switch for Run3 and beyond #29617

Merged

cmsbuild added reconstruction-approved and removed reconstruction-pending labels May 4, 2020

cmsbuild added fully-signed hlt-approved and removed hlt-pending pending-signatures labels May 5, 2020

cmsbuild added orp-approved and removed orp-pending labels May 5, 2020

cmsbuild merged commit 968b6b1 into cms-sw:master May 5, 2020

guitargeek deleted the ElectronNHitSeedProducer_1_squashed branch May 5, 2020 08:21

guitargeek mentioned this pull request May 12, 2020

Continue work on electron seeding convergence #29815

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ElectronNHitSeedProducer modernization #29512

ElectronNHitSeedProducer modernization #29512

guitargeek commented Apr 17, 2020

cmsbuild commented Apr 17, 2020

cmsbuild commented Apr 17, 2020

cmsbuild commented Apr 17, 2020

slava77 commented Apr 17, 2020

cmsbuild commented Apr 17, 2020 •

edited

cmsbuild commented Apr 17, 2020

cmsbuild commented Apr 17, 2020

Martin-Grunewald commented Apr 18, 2020

guitargeek commented Apr 18, 2020

cmsbuild commented Apr 27, 2020

cmsbuild commented Apr 27, 2020

cmsbuild commented Apr 27, 2020

perrotta commented May 4, 2020

Martin-Grunewald commented May 5, 2020

cmsbuild commented May 5, 2020

silviodonato commented May 5, 2020

Navigation Menu

ElectronNHitSeedProducer modernization #29512

ElectronNHitSeedProducer modernization #29512

Conversation

guitargeek commented Apr 17, 2020

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 Apr 17, 2020

cmsbuild commented Apr 17, 2020

cmsbuild commented Apr 17, 2020

slava77 commented Apr 17, 2020

cmsbuild commented Apr 17, 2020 • edited

cmsbuild commented Apr 17, 2020

cmsbuild commented Apr 17, 2020

Martin-Grunewald commented Apr 18, 2020

guitargeek commented Apr 18, 2020

cmsbuild commented Apr 27, 2020

cmsbuild commented Apr 27, 2020

cmsbuild commented Apr 27, 2020

perrotta commented May 4, 2020

Martin-Grunewald commented May 5, 2020

cmsbuild commented May 5, 2020

silviodonato commented May 5, 2020

cmsbuild commented Apr 17, 2020 •

edited