Muon FSR recovery for miniaod and nanoaod #28118

arizzi · 2019-10-05T14:41:01Z

PR description:

This PR adds the producers for FSR recovery needed for next round of NanoAOD.
As the producer are generic enough to run also on top of MiniAOD they have been included as discussed with muon POG (https://its.cern.ch/jira/browse/CMSMUONS-300) outside the PhysicsTools/NanoAOD package

The nanoaod enabling code is not included in this PR and will be integrated once this enters cmssw.
A version of this PR with the nanoaod code (actually just some configuration) has been tested here:
cms-nanoAOD#413

PR validation:

Tests have been performed in the nanoaod PR linked above, no standard workflow is currently affected (inclusion during miniaod production step has not yet being discussed in XPOG)

@NTrevisani @jorieger @peruzzim

@NTrevisani

@NTrevisani) code

…topic

…ion of out photons, fix idx

As #39 without NANOAOD code

cmsbuild · 2019-10-05T14:41:26Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-10-05T14:48:25Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28118/12143

This PR adds an extra 32KB to repository

cmsbuild · 2019-10-05T14:48:55Z

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

It involves the following packages:

DataFormats/PatCandidates
MuonAnalysis/MuonAssociators

@perrotta, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@gouskos, @calderona, @abbiendi, @jhgoh, @bellan, @HuguesBrun, @rovere, @hatakeyamak, @gpetruc, @battibass, @trocino, @peruzzim, @folguera, @cbernet 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

peruzzim · 2019-10-05T22:07:17Z

please test

cmsbuild · 2019-10-05T22:07:42Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2803/console Started: 2019/10/06 00:08

cmsbuild · 2019-10-05T23:56:44Z

-1

Tested at: 46f1bfa

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-33ee8e/2803/summary.html

I found follow errors while testing this PR

Failed tests: AddOn

AddOn:

I found errors in the following addon tests:

cmsDriver.py TTbar_13TeV_TuneCUETP8M1_cfi --conditions auto:run2_mc_l1stage1 --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,EI,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot NominalCollision2015 --era Run2_25ns : FAILED - time: date Sun Oct 6 01:32:55 2019-date Sun Oct 6 01:28:03 2019 s - exit: 35584

cmsbuild · 2019-10-05T23:56:46Z

Comparison job queued.

cmsbuild · 2019-10-06T01:31:43Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-33ee8e/2803/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: 2961049
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2960706
DQMHistoTests: Total skipped: 341
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

cmsbuild · 2019-10-09T21:14:42Z

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

cmsbuild · 2019-10-09T21:22:12Z

The tests are being triggered in jenkins.
Tested with other pull request(s) #28122
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2859/console Started: 2019/10/09 23:22

cmsbuild · 2019-10-09T23:07:57Z

+1
Tested at: 93a79e1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ffc02d/2859/summary.html

cmsbuild · 2019-10-09T23:08:03Z

Comparison job queued.

cmsbuild · 2019-10-10T05:06:49Z

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

Comparison Summary:

No significant changes to the logs found
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 15 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2961053
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2960710
DQMHistoTests: Total skipped: 341
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 1.285 KiB( 33 files compared)
DQMHistoSizes: changed ( 1325.7 ): 1.285 KiB Physics/NanoAODDQM
Checked 147 log files, 16 edm output root files, 34 DQM output files

fabiocos · 2019-10-10T07:53:13Z

@slava77 your suggestion has been implemented, providing a successful run of the nanoAOD code (see #28122 and I did a private cross check). As you had already signed the PR and nothing apart your suggestion has been modified, I will consider this as ok and integrate it

@peruzzim @santocch FYI

fabiocos · 2019-10-10T07:54:06Z

+1

fabiocos · 2019-10-10T08:06:09Z

merge

santocch · 2019-10-10T08:19:48Z

+1

Dr15Jones · 2019-10-20T13:53:05Z

MuonAnalysis/MuonAssociators/plugins/MuonFSRProducer.cc

+      pat::PackedCandidateRef pfcandRef = pat::PackedCandidateRef(pfcands, iter_pf - pfcands->begin());
+
+      for (auto electrons_iter = electrons->begin(); electrons_iter != electrons->end(); ++electrons_iter) {
+        for (auto itr = electrons_iter->associatedPackedPFCandidates().begin();


This does not work. associatedPackedPFCandidates() returns a temporary object

cmssw/DataFormats/PatCandidates/interface/Electron.h

Line 260 in a41ba6c

edm::RefVector<pat::PackedCandidateCollection> associatedPackedPFCandidates() const;

so the calls to electrons_iter->associatedPackedPFCandidates().begin() and electrons_iter->associatedPackedPFCandidates().begin() are actually returning iterators to two different containers (both of which are gone during the iteration of the loop).

This was found by the address sanitizer
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc820/CMSSW_11_0_ASAN_X_2019-10-18-2300/pyRelValMatrixLogs/run/250202.118_ProdTTbar_13_pmx25ns+ProdTTbar_13UP18+DIGIPRMXUP18_PROD_PU25+RECOPRMXUP18PROD_PU25+NANOEDMMC2018_PROD/step4_ProdTTbar_13_pmx25ns+ProdTTbar_13UP18+DIGIPRMXUP18_PROD_PU25+RECOPRMXUP18PROD_PU25+NANOEDMMC2018_PROD.log#/

shouldn't it loop forever then?

It's actually undefined behavior. I'm guessing that the two temporaries just happen to be made at the same point on the stack. This causes a lucky accident that it just happens to work for now.

ok then we may experience random crashes in large production.
@peruzzim I'm not sure if the nanov6 campaign was already submitted, but I'd suggest to fix this (just having a local copy of the vector)

I think a possibly better solution would be to switch to using ranged for loop

for(auto const& cand: electrons_iter->associatedPackedPFCandidates()) {

The language guarantees that the temporary container returned from the call to electrons_iter->associatedPackedPFCandidates() will be kept around for the lifetime of the for loop.

implemented this solution in #28221

jfernan2 · 2023-09-18T08:21:44Z

+1

cmsbuild · 2023-09-18T08:22:06Z

Pull request #28118 was updated. @simonepigazzini, @vlimant can you please check and sign again.

arizzi and others added 8 commits October 4, 2019 12:15

Muon FSR recovery producer based on Oliver Rieger and Nicolo Trevisani (

845d54b

@NTrevisani) code

Merged nanoAOD-FSR-11_0_0_pre9 from repository arizzi with cms-merge-…

1d4c949

…topic

More configuration parameters, add defaults in description, preselect…

47599f4

…ion of out photons, fix idx

Fixes discussed in review

ed409dc

Refactor FSR photon producer and associator

3df3271

code format

5714cc4

Test for product ID in electron footprint cleaning for FSR photons

f92550a

Merge pull request #40 from peruzzim/FSRphotons_110X_arizzi_noNano

46f1bfa

As #39 without NANOAOD code

cmsbuild added this to the CMSSW_11_0_X milestone Oct 5, 2019

cmsbuild added analysis-pending code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Oct 5, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 5, 2019

cmsbuild added tests-started and removed tests-pending labels Oct 5, 2019

cmsbuild added tests-rejected and removed tests-started labels Oct 5, 2019

cmsbuild added the code-checks-approved label Oct 9, 2019

cmsbuild added requires-external tests-started and removed tests-pending labels Oct 9, 2019

cmsbuild added tests-approved and removed tests-started labels Oct 9, 2019

cmsbuild added comparison-available and removed comparison-pending labels Oct 10, 2019

cmsbuild added orp-approved and removed orp-pending labels Oct 10, 2019

cmsbuild merged commit 52da0de into cms-sw:master Oct 10, 2019

cmsbuild added analysis-approved and removed analysis-pending labels Oct 10, 2019

fabiocos mentioned this pull request Oct 14, 2019

plugin for muon FSR recovery #27836

Closed

Dr15Jones reviewed Oct 20, 2019

View reviewed changes

peruzzim mentioned this pull request Oct 20, 2019

Fix electron PFCandidates RefVector usage in MuonFSRProducer #28221

Merged

cmsbuild added reconstruction-approved xpog-pending and removed reconstruction-pending labels Sep 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Muon FSR recovery for miniaod and nanoaod #28118

Muon FSR recovery for miniaod and nanoaod #28118

arizzi commented Oct 5, 2019

cmsbuild commented Oct 5, 2019

cmsbuild commented Oct 5, 2019

cmsbuild commented Oct 5, 2019

peruzzim commented Oct 5, 2019

cmsbuild commented Oct 5, 2019 •

edited

cmsbuild commented Oct 5, 2019

cmsbuild commented Oct 5, 2019

cmsbuild commented Oct 6, 2019

cmsbuild commented Oct 9, 2019

cmsbuild commented Oct 9, 2019 •

edited

cmsbuild commented Oct 9, 2019

cmsbuild commented Oct 9, 2019

cmsbuild commented Oct 10, 2019

fabiocos commented Oct 10, 2019

fabiocos commented Oct 10, 2019

fabiocos commented Oct 10, 2019

santocch commented Oct 10, 2019

Dr15Jones Oct 20, 2019

arizzi Oct 20, 2019

Dr15Jones Oct 20, 2019 •

edited

arizzi Oct 20, 2019

Dr15Jones Oct 20, 2019

peruzzim Oct 20, 2019

jfernan2 commented Sep 18, 2023

cmsbuild commented Sep 18, 2023

Muon FSR recovery for miniaod and nanoaod #28118

Muon FSR recovery for miniaod and nanoaod #28118

Conversation

arizzi commented Oct 5, 2019

PR description:

PR validation:

cmsbuild commented Oct 5, 2019

cmsbuild commented Oct 5, 2019

cmsbuild commented Oct 5, 2019

peruzzim commented Oct 5, 2019

cmsbuild commented Oct 5, 2019 • edited

cmsbuild commented Oct 5, 2019

cmsbuild commented Oct 5, 2019

cmsbuild commented Oct 6, 2019

cmsbuild commented Oct 9, 2019

cmsbuild commented Oct 9, 2019 • edited

cmsbuild commented Oct 9, 2019

cmsbuild commented Oct 9, 2019

cmsbuild commented Oct 10, 2019

fabiocos commented Oct 10, 2019

fabiocos commented Oct 10, 2019

fabiocos commented Oct 10, 2019

santocch commented Oct 10, 2019

Dr15Jones Oct 20, 2019

Choose a reason for hiding this comment

arizzi Oct 20, 2019

Choose a reason for hiding this comment

Dr15Jones Oct 20, 2019 • edited

Choose a reason for hiding this comment

arizzi Oct 20, 2019

Choose a reason for hiding this comment

Dr15Jones Oct 20, 2019

Choose a reason for hiding this comment

peruzzim Oct 20, 2019

Choose a reason for hiding this comment

jfernan2 commented Sep 18, 2023

cmsbuild commented Sep 18, 2023

cmsbuild commented Oct 5, 2019 •

edited

cmsbuild commented Oct 9, 2019 •

edited

Dr15Jones Oct 20, 2019 •

edited