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

Expand track information in lostTracks collection #32242

Merged

Conversation

mbluj
Copy link
Contributor

@mbluj mbluj commented Nov 23, 2020

PR description:

This PR expands detailed track information stored in the lostTracks collection (pat::PackedCandiates) in miniAOD. The following changes are implemented:

  • Track to PV association is preformed in similar way as for tracks contained in packedPFCandidates collection (packed PF particles);
  • Detailed track information is stored down to 0.5GeV instead of current 0.95GeV as done for packed PF particles (packedPFCandidates). Track covariance packing schemas are adapted from producer of the packed PF particle collection for more effective packing.

MiniAOD size changes were estimated with 1k of DY->LL evens with 2018 conditions and it is as follows:

  • size increase of patPackedCandidates_lostTracks__PAT: 23%
  • size increase of full miniAOD sample: 0.2%
  • 1/4 if the increase comes from updated PV association information, while remaining 3/4 from lowering threshold to store detailed track info.

PR validation:

Tested with standard miniAOD workflow.
Matrix tests (runTheMatrix.py -l limited -i all --ibeos) successful.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32242/19997

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

PhysicsTools/PatAlgos

@perrotta, @jpata, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @JyothsnaKomaragiri, @ahinzmann, @smoortga, @schoef, @mmarionncern, @jdamgov, @jdolen, @nhanvtran, @gkasieczka, @clelange, @emilbols, @hatakeyamak, @ferencek, @gpetruc, @andrzejnovak, @mariadalfonso, @seemasharmafnal 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

@slava77
Copy link
Contributor

slava77 commented Nov 23, 2020

@mbluj
is this planned to be backported to 10_6_X for the UL reminiAOD?

was this update already discussed with XPOG?

@cms-sw/xpog-l2

@slava77
Copy link
Contributor

slava77 commented Nov 23, 2020

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 23, 2020

The tests are being triggered in jenkins.

@mbluj
Copy link
Contributor Author

mbluj commented Nov 23, 2020

@mbluj
is this planned to be backported to 10_6_X for the UL reminiAOD?

No it is not planned, but can be done if requested (however it looks be late for it)

was this update already discussed with XPOG?

@cms-sw/xpog-l2

No, it has been not discussed. I can prepare a few slides on this if it will help (@cms-sw/xpog-l2 please let me know). However, I think that changes are rather small. Regardless, one of goals of the changes is it improve tau (re)reco with miniAOD inputs.

@cmsbuild
Copy link
Contributor

+1
Tested at: 02d8fab
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-92a09a/10951/summary.html
CMSSW: CMSSW_11_2_X_2020-11-23-1100
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

Comparison job queued.

@perrotta
Copy link
Contributor

@mbluj did the sample used to test the size include PU? Otherwise size and timing should be reassessed with a PU sample
In any case, let me assign also xpog here, so that they can sign-up for the additional miniAOD needs

@perrotta
Copy link
Contributor

assign xpog

@cmsbuild
Copy link
Contributor

New categories assigned: xpog

@fgolf,@mariadalfonso,@gouskos you have been requested to review this Pull request/Issue and eventually sign? Thanks

@perrotta
Copy link
Contributor

Moreover: about the possible backport for the re-miniAOD of the legacy samples, this has to be decided now. Again, I'd let you (the tau group) and @cms-sw/xpog-l2 to decide. Please, keep in mind that lostTracks cannot be re-made based on miniAOD, and therefore if you decide to include this update sometime in the future, you cannot rely on one of the next re-miniAOD based on miniAOD, only on a further major one

@mbluj
Copy link
Contributor Author

mbluj commented Nov 23, 2020

@mbluj did the sample used to test the size include PU? Otherwise size and timing should be reassessed with a PU sample
In any case, let me assign also xpog here, so that they can sign-up for the additional miniAOD needs

Yes, it was tested with sample with 2018 PU (RunIISummer19UL18RECO campaign).

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 26, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+1
Tested at: 47972cf
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-92a09a/11063/summary.html
CMSSW: CMSSW_11_2_X_2020-11-25-2300
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 231 differences found in the comparisons
  • DQMHistoTests: Total files compared: 36
  • DQMHistoTests: Total histograms compared: 2747014
  • DQMHistoTests: Total failures: 1308
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2745684
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
  • Checked 153 log files, 34 edm output root files, 36 DQM output files

@perrotta
Copy link
Contributor

+1

  • lostTracks stored down to 0.5 GeV, with some modification in the track to PV associationa, as in the PR description
  • miniAOD size increase in a DY sample with Pu was quantified to be 23% for lostTracks, which corresponds to an overall increas of the full miniAOD sample of 0.2%
  • Similar performance check was done with wf 10224 (TTbar with PU): the timing was fount to be fully comparable, while the miniAOD size increase is of the order of 19% for lostTracks, which corresponds to 0.14% overall on the whole miniAOD
  • Jenkins tests pass and the differences observed are consequential to the larger number of saved lostTracks
  • Tests runs with the useLegacySetup flag set to false did not show any difference wrt the baseline

@mariadalfonso
Copy link
Contributor

+xpog

I link here the presentation at the xPOG meeting
https://indico.cern.ch/event/971101/#5-tau-reco-in-aod-vs-miniaod

@santocch
Copy link

+1

@cmsbuild
Copy link
Contributor

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

@qliphy
Copy link
Contributor

qliphy commented Nov 26, 2020

+1

@cmsbuild cmsbuild merged commit d3acff5 into cms-sw:master Nov 27, 2020
@mbluj mbluj deleted the CMSSW_11_2_X_tau-pog_lostTrackMinMod branch October 10, 2023 10:10
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

8 participants