Fix thread safety of EgammaHLTPixelMatchElectronProducers

[Dr15Jones]

EgammaHLTPixelMatchElectronAlgo is not thread safe and can not be used as a member function of a edm::global module. Converting EgammaHLTPixelMatchElectronProducers to a stream module fixes the problem.
Also did some simple code modernization of the module.

[cmsbuild]

The code-checks are being triggered in jenkins.

[Dr15Jones]

This problem showed up as a double delete in the ASAN report.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25589/7832

[cmsbuild]

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

RecoEgamma/EgammaHLTProducers

@Martin-Grunewald, @cmsbuild, @fwyzard can you please review it and eventually sign? Thanks.
@Sam-Harper, @battibass, @jainshilpi, @lgray, @calderona, @HuguesBrun, @varuns23, @folguera 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

[Dr15Jones]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32418/console Started: 2019/01/05 16:21

[cmsbuild]

+1
Tested at: 9a5dab5
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25589/32418/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 33
• DQMHistoTests: Total histograms compared: 3153717
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3153512
• DQMHistoTests: Total skipped: 204
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
• Checked 137 log files, 14 edm output root files, 33 DQM output files

[Martin-Grunewald]

+1

[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)

[fwyzard]

Wait, wouldn't it be better to fix EgammaHLTPixelMatchElectronAlgo and make it global-module-safe instead ?

At the same time, wouldn't it be better to have the consumes calls and the corresponding tokens would be better in the same class ?

[Sam-Harper]

@fwyzard , it would probably be better to delete the code entirely, its not run in the HLT any more since 2016 and the new code should be able to reproduce the old code behaviour (although this was not validated and would need to be first).

[Dr15Jones]

@Sam-Harper this code is run in the nightly RelVals which is where the problem was uncovered. If it is to be removed, the effected workflows would need to be updated with the new module that does the same work.

[Sam-Harper]

Thanks @Dr15Jones . Must be running a pre-2017 menu. Indeed this is part of the reason why this module hasnt been removed yet (and thanks for fixing btw). Something to discuss with the HLT group what we want to do here (leave an old piece of fairly nasty code which has a maintenance burden so we can continue to run old menus in new releases or update those old menus to use the new code).

[fwyzard]

@Sam-Harper thanks for the comment.
Fine by me to fix as suggested by @Dr15Jones , and I agree the TSG should put forward a policy for removing obsolete menus and modules.

[fabiocos]

+1

[Martin-Grunewald]

@Sam-Harper
Well,
hltEgammaGsfElectrons = cms.EDProducer( "EgammaHLTPixelMatchElectronProducers",
and
hltEgammaGsfElectronsPPOnAA = cms.EDProducer( "EgammaHLTPixelMatchElectronProducers",
is still used in many paths in current GRun and HIon menus:

GRun:
HLT_DoubleMu5_Upsilon_DoubleEle3_CaloIdL_TrackIdL_v4 HLT_DoubleMu3_DoubleEle7p5_CaloIdL_TrackIdL_Upsilon_v4 HLT_DoubleEle24_eta2p1_WPTight_Gsf_v7 HLT_DoubleEle8_CaloIdM_TrackIdM_Mass8_DZ_PFHT350_v20 HLT_DoubleEle8_CaloIdM_TrackIdM_Mass8_PFHT350_v20 HLT_Ele20_WPTight_Gsf_v6 HLT_Ele15_WPLoose_Gsf_v3 HLT_Ele17_WPLoose_Gsf_v3 HLT_Ele20_WPLoose_Gsf_v6 HLT_Ele20_eta2p1_WPLoose_Gsf_v6 HLT_Ele27_WPTight_Gsf_v16 HLT_Ele28_WPTight_Gsf_v1 HLT_Ele30_WPTight_Gsf_v1 HLT_Ele32_WPTight_Gsf_v15 HLT_Ele35_WPTight_Gsf_v9 HLT_Ele35_WPTight_Gsf_L1EGMT_v5 HLT_Ele38_WPTight_Gsf_v9 HLT_Ele40_WPTight_Gsf_v9 HLT_Ele32_WPTight_Gsf_L1DoubleEG_v9 HLT_Ele24_eta2p1_WPTight_Gsf_LooseChargedIsoPFTauHPS30_eta2p1_CrossL1_v1 HLT_Ele24_eta2p1_WPTight_Gsf_MediumChargedIsoPFTauHPS30_eta2p1_CrossL1_v1 HLT_Ele24_eta2p1_WPTight_Gsf_TightChargedIsoPFTauHPS30_eta2p1_CrossL1_v1 HLT_Ele24_eta2p1_WPTight_Gsf_LooseChargedIsoPFTauHPS30_eta2p1_TightID_CrossL1_v1 HLT_Ele24_eta2p1_WPTight_Gsf_MediumChargedIsoPFTauHPS30_eta2p1_TightID_CrossL1_v1 HLT_Ele24_eta2p1_WPTight_Gsf_TightChargedIsoPFTauHPS30_eta2p1_TightID_CrossL1_v1 HLT_Ele24_eta2p1_WPTight_Gsf_LooseChargedIsoPFTau30_eta2p1_CrossL1_v13 HLT_Mu8_DiEle12_CaloIdL_TrackIdL_DZ_v18 HLT_Mu8_DiEle12_CaloIdL_TrackIdL_v18 HLT_Mu8_Ele8_CaloIdM_TrackIdM_Mass8_PFHT350_DZ_v19 HLT_Mu8_Ele8_CaloIdM_TrackIdM_Mass8_PFHT350_v19 HLT_Mu8_TrkIsoVVL_Ele23_CaloIdL_TrackIdL_IsoVL_DZ_v13 HLT_Mu8_TrkIsoVVL_Ele23_CaloIdL_TrackIdL_IsoVL_DZ_PFDiJet30_v1 HLT_Mu8_TrkIsoVVL_Ele23_CaloIdL_TrackIdL_IsoVL_DZ_CaloDiJet30_v1 HLT_Mu8_TrkIsoVVL_Ele23_CaloIdL_TrackIdL_IsoVL_DZ_PFDiJet30_PFBtagDeepCSV_1p5_v1 HLT_Mu8_TrkIsoVVL_Ele23_CaloIdL_TrackIdL_IsoVL_DZ_CaloDiJet30_CaloBtagDeepCSV_1p5_v1 HLT_Mu8_TrkIsoVVL_Ele23_CaloIdL_TrackIdL_IsoVL_v11 HLT_Ele15_Ele8_CaloIdL_TrackIdL_IsoVL_v3 HLT_Ele23_Ele12_CaloIdL_TrackIdL_IsoVL_DZ_v19 HLT_Ele23_Ele12_CaloIdL_TrackIdL_IsoVL_v19 HLT_Mu23_TrkIsoVVL_Ele12_CaloIdL_TrackIdL_IsoVL_DZ_v15 HLT_Mu23_TrkIsoVVL_Ele12_CaloIdL_TrackIdL_IsoVL_v7 HLT_Mu12_TrkIsoVVL_Ele23_CaloIdL_TrackIdL_IsoVL_v7 HLT_Mu12_TrkIsoVVL_Ele23_CaloIdL_TrackIdL_IsoVL_DZ_v15 HLT_Ele30_eta2p1_WPTight_Gsf_CentralPFJet35_EleCleaned_v13 HLT_Ele28_eta2p1_WPTight_Gsf_HT150_v13 HLT_Ele28_HighEta_SC20_Mass55_v13 HLT_Ele15_IsoVVVL_PFHT450_CaloBTagDeepCSV_4p5_v8 HLT_Ele15_IsoVVVL_PFHT450_PFMET50_v16 HLT_Ele15_IsoVVVL_PFHT450_v16 HLT_Ele15_IsoVVVL_PFHT600_v20 HLT_Ele50_IsoVVVL_PFHT450_v16 HLT_DiMu4_Ele9_CaloIdL_TrackIdL_DZ_Mass3p8_v17 HLT_DiMu9_Ele9_CaloIdL_TrackIdL_DZ_v17 HLT_DiMu9_Ele9_CaloIdL_TrackIdL_v17 HLT_Ele8_CaloIdL_TrackIdL_IsoVL_PFJet30_v16 HLT_Ele12_CaloIdL_TrackIdL_IsoVL_PFJet30_v18 HLT_Ele15_CaloIdL_TrackIdL_IsoVL_PFJet30_v3 HLT_Ele23_CaloIdL_TrackIdL_IsoVL_PFJet30_v18 HLT_Ele8_CaloIdM_TrackIdM_PFJet30_v18 HLT_Ele17_CaloIdM_TrackIdM_PFJet30_v16 HLT_Ele23_CaloIdM_TrackIdM_PFJet30_v18 HLT_Ele50_CaloIdVT_GsfTrkIdT_PFJet165_v18 HLT_Ele115_CaloIdVT_GsfTrkIdT_v14 HLT_Ele135_CaloIdVT_GsfTrkIdT_v7 HLT_Ele145_CaloIdVT_GsfTrkIdT_v8 HLT_Ele200_CaloIdVT_GsfTrkIdT_v8 HLT_Ele250_CaloIdVT_GsfTrkIdT_v13 HLT_Ele300_CaloIdVT_GsfTrkIdT_v13 DST_HT410_PFScouting_v16 DST_HT410_BTagScouting_v16 DST_ZeroBias_BTagScouting_v15 DST_ZeroBias_CaloScouting_PFScouting_v14 DST_CaloJet40_BTagScouting_v15 DST_CaloJet40_CaloScouting_PFScouting_v15 DST_CaloJet40_CaloBTagScouting_v14 DST_L1HTT_BTagScouting_v15 DST_L1HTT_CaloScouting_PFScouting_v15 DST_L1HTT_CaloBTagScouting_v14 DST_L1DoubleMu_BTagScouting_v16 DST_L1DoubleMu_CaloScouting_PFScouting_v15 DST_DoubleMu3_noVtx_Mass10_PFScouting_v3 MC_Ele5_WPTight_Gsf_v8 MC_Ele15_Ele10_CaloIdL_TrackIdL_IsoVL_DZ_v15 HLT_Ele16_Ele12_Ele8_CaloIdL_TrackIdL_v9 

HIon:
HLT_HIL1Mu3Eta2p5_Ele10Gsf_v1 HLT_HIL1Mu5Eta2p5_Ele10Gsf_v1 HLT_HIL1Mu7Eta2p5_Ele10Gsf_v1 HLT_HIEle10Gsf_v1 HLT_HIEle10Gsf_PuAK4CaloJet40Eta2p1_v1 HLT_HIEle10Gsf_PuAK4CaloJet60Eta2p1_v1 HLT_HIEle10Gsf_PuAK4CaloJet80Eta2p1_v1 HLT_HIEle10Gsf_PuAK4CaloJet100Eta2p1_v1 HLT_HIL1Mu3Eta2p5_Ele15Gsf_v1 HLT_HIL1Mu5Eta2p5_Ele15Gsf_v1 HLT_HIL1Mu7Eta2p5_Ele15Gsf_v1 HLT_HIEle15Gsf_v1 HLT_HIEle15Gsf_PuAK4CaloJet40Eta2p1_v1 HLT_HIEle15Gsf_PuAK4CaloJet60Eta2p1_v1 HLT_HIEle15Gsf_PuAK4CaloJet80Eta2p1_v1 HLT_HIEle15Gsf_PuAK4CaloJet100Eta2p1_v1 HLT_HIL1Mu3Eta2p5_Ele20Gsf_v1 HLT_HIEle20Gsf_v1 HLT_HIEle20Gsf_PuAK4CaloJet40Eta2p1_v1 HLT_HIEle20Gsf_PuAK4CaloJet60Eta2p1_v1 HLT_HIEle20Gsf_PuAK4CaloJet80Eta2p1_v1 HLT_HIEle20Gsf_PuAK4CaloJet100Eta2p1_v1 HLT_HIL1Mu5Eta2p5_Ele20Gsf_v1 HLT_HIL1Mu7Eta2p5_Ele20Gsf_v1 HLT_HIEle30Gsf_v1 HLT_HIEle40Gsf_v1 HLT_HIEle50Gsf_v1 HLT_HIEle15Ele10Gsf_v1 HLT_HIEle15Ele10GsfMass50_v1 HLT_HIDoubleEle10Gsf_v1 HLT_HIDoubleEle10GsfMass50_v1 HLT_HIDoubleEle15Gsf_v1 HLT_HIDoubleEle15GsfMass50_v1 

[Sam-Harper]

Hi @Martin-Grunewald

Thank you for the correction. Sorry, I spoke to fast, I thought this was the pixel matching but is the maker of pixel matched electrons. Its not entirely clear to me that any path actually uses it though but it is run.