Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store TrackExtra, TrackingRecHits, and SiPixel/StripClusters for muons in AOD+MINIAOD (Self-contained version without framework thinning) #31217

Merged
merged 20 commits into from Sep 4, 2020

Conversation

bendavid
Copy link
Contributor

@bendavid bendavid commented Aug 24, 2020

Precision measurements using muons might want to be able to refit the muon tracks (and the momentum scale corrections for W mass are moving in this direction).

This PR adds thinned collections of TrackExtra and the corresponding TrackingRecHits and Strip/Pixel clusters to AOD for those tracks which are associated to muon::bestTrack() with some cuts applied to the muons.
(this means that for most muons with pt<200GeV only the pixel and strip rechits are stored and not the muon chamber ones, because bestTrack points to the inner track unless the muon is purely standalone). This behaviour could be modified if desired to store some additional trackExtra/hits for the global track.

The cut applied to the muons to store the extra information in AOD is pt>3.0 GeV.

A further reduced set of collections is stored in MINIAOD, applying a 4.5 GeV cut on top of the selectedPatMuons (such that the extra cuts in

process.selectedPatMuons.cut = cms.string("pt > 5 || isPFMuon || (pt > 3 && (isGlobalMuon || isStandAloneMuon || numberOfMatches > 0 || muonID('RPCMuLoose')))")
are implicitly included)

This is a fallback alternative to #30544 in case the needed framework functionality is not integrated quickly enough to backport this to 10_6 in time for the restart of the UL MC campaigns. (Since the AOD event content is modified it would be highly desirable to have this in for consistency purposes)

This PR does not touch any existing producers in the RECO sequence and only adds additional collections. Small modifications are made to the PATMuonSlimmer in the MINIAOD sequence.

Tested using 11834.0 (TTbar 2021 realistic with PU Run3_Flat55To75_PoissonOOTPU) 500 events.

The additional collections in AOD are:

Branch Name | Average Uncompressed Size (Bytes/Event) | Average Compressed Size (Bytes/Event) 
recoTrackExtras_muonReducedTrackExtras__RECO. 951.718 619.05
SiStripClusteredmNewDetSetVector_muonReducedTrackExtras__RECO. 704.88 312.796
SiPixelClusteredmNewDetSetVector_muonReducedTrackExtras__RECO. 521.332 288.952
TrackingRecHitsOwned_muonReducedTrackExtras__RECO. 2374.61 268.714
recoTrackExtrasedmAssociation_muonReducedTrackExtras__RECO. 9695.24 135.65

(and the total size per event increases from 672 kB to 674 kB for a 0.25% increase)

The additional collections in MINIAOD are:

Branch Name | Average Uncompressed Size (Bytes/Event) | Average Compressed Size (Bytes/Event) 
TrackingRecHitsOwned_slimmedMuonTrackExtras__RECO. 1919.71 387.802
recoTrackExtras_slimmedMuonTrackExtras__RECO. 639.646 303.8
SiStripClusteredmNewDetSetVector_slimmedMuonTrackExtras__RECO. 368.938 157.746
SiPixelClusteredmNewDetSetVector_slimmedMuonTrackExtras__RECO. 235.028 100.002

(and the total size per event increases from 101.5 kB to 102.4 kB for a 0.9% increase)

Some drawbacks of this approach with respect to the leveraging of framework thinning and slimming as in #30544:
-code is based on recursive rekeying and is therefore harder to maintain
-some duplication of reco::TrackExtras which are already stored in AOD (but need to be duplicated in order to have consistent rekeying of references to TrackingRecHits in the reduced collection)
-additional information is not easily accessible at AOD level, because the final reference rekeying only happens during the production of the slimmedMuons at MINIAOD level (but the association maps are present, so it is possible at least)

Some open points with this PR:

  1. information stored is incomplete for phase 2 (but phase 2 workflows should still run)
  2. edm::Association is maybe not the most efficient for this purpose (see large uncompressed size, though it compresses fairly well given that most of the references for generalTracks are null)
  3. edm::Association<reco::TrackExtraCollection> required adding additional dictionaries in DataFormats/TrackReco (so this will produce errors when reading the corresponding AOD files with an older release unless dropping those on input)
  4. Currently the use of the additional information at the MINIAOD step is DISABLED for Run 2 UL, since the needed collections are not present in the AOD. If we manage to get this in before the restart of production in the latest 10_6 release, then the process modifiers for Run 2 UL will need to be split into "new" and "old" variants, with this disabled for Run2ULold, but left enabled for Run2ULnew

Point 1) I think can be dealt with in a future PR
For points 2) and 3) it may be ok as is, but open to suggestions for alternatives to edm::Association<reco::TrackExtraCollection> for this use case (associating the original TrackExtra references to the reduced collection)

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31217/17870

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31217/17871

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @bendavid (Josh Bendavid) for master.

It involves the following packages:

DataFormats/PatCandidates
DataFormats/TrackReco
PhysicsTools/PatAlgos
RecoMuon/Configuration
RecoMuon/MuonIdentification

@perrotta, @jpata, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @abbiendi, @emilbols, @echapon, @ahinzmann, @peruzzim, @seemasharmafnal, @mmarionncern, @JyothsnaKomaragiri, @makortel, @smoortga, @jhgoh, @jdolen, @HuguesBrun, @cericeci, @ferencek, @trocino, @rociovilar, @rovere, @VinInn, @jdamgov, @bellan, @nhanvtran, @gkasieczka, @schoef, @andrzejnovak, @clelange, @riga, @Fedespring, @calderona, @hatakeyamak, @cbernet, @gpetruc, @mariadalfonso, @folguera this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@VinInn
Copy link
Contributor

VinInn commented Aug 24, 2020

I may have been missing some details. Would not be easier and more robust to just make a deep copy?

@bendavid
Copy link
Contributor Author

Deep copy of what?

The needed information is spread over reco::Track, reco::TrackExtra, TrackingRecHit, SiPixelCluster, SiStripCluster, and these objects are linked together by edm::Ref, so the only two possibilities I'm aware of are

  1. Separately copy each collection and recursively rekey (as done here)
  2. Use framework thinning functionality to separately copy each collection and rely on the framework to forward the references automatically (as done in [RFC] Store TrackExtra, TrackingRecHits, and SiPixel/StripClusters for muons in AOD+MINIAOD #30544 but needing also some non-trivial extensions to the framework thinning functionality to be viable)

Open to other alternatives of course.

@VinInn
Copy link
Contributor

VinInn commented Aug 24, 2020

track, extra and hits can be deep copied using
https://cmssdt.cern.ch/lxr/source/RecoTracker/FinalTrackSelectors/src/TrackCollectionCloner.cc?v=CMSSW_11_2_X_2020-08-19-2300

so what is left is to copy clusters and to rekey omni-refs in hits
(I assume one does not care about seeds)

I admit that DetSetVector is a pretty complex container for thinning and a simpler vector and vector would be easier to fill and handle...
BUT the Ref to cluster is strongly related to DetSetVector even if at the end it is just locating the cluster in a vector

@makortel
Copy link
Contributor

I admit that DetSetVector is a pretty complex container for thinning and a simpler vector and vector would be easier to fill and handle...

edmNew::DetSetVector wasn't really that complicated in the end (at least for the "thinning" in framework), and #31100 supports it.

@civanch
Copy link
Contributor

civanch commented Sep 3, 2020

+1

@kpedro88
Copy link
Contributor

kpedro88 commented Sep 3, 2020

+upgrade

@silviodonato
Copy link
Contributor

+operations

@silviodonato
Copy link
Contributor

kind reminder @cms-sw/analysis-l2 @cms-sw/pdmv-l2

@chayanit
Copy link

chayanit commented Sep 4, 2020

+1

@silviodonato
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 0666b7e into cms-sw:master Sep 4, 2020
@santocch
Copy link

santocch commented Sep 5, 2020

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 5, 2020

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 be automatically merged.

slava77 pushed a commit to slava77/cms-bot that referenced this pull request Sep 9, 2020
- for slimmed/reduced muon clusters/extras from cms-sw/cmssw#31217: add some basic plots or `reco::TrackExtra` and the associated pixel/strip clusters
    - comes with some refactoring of existing plotting methods for the tracker clusters
- for slimmedHcalRecHits from cms-sw/cmssw#31375 : basic plots for HBHE/HO/HF hits are added
    - comes with some refactoring of existing plotting methods for the calo rec hits
- add a plot for hbhe rechits chi2 after some minimal energy cut
- add pat Electron and Muon BS2D and PV2D plots
- adjust the stats text and the exponent label offset for x and y axis plots to avoid/minimize overlapping or invisible text
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet