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

Add V0s to miniaod #18300

Merged
merged 1 commit into from
Apr 21, 2017
Merged

Add V0s to miniaod #18300

merged 1 commit into from
Apr 21, 2017

Conversation

arizzi
Copy link
Contributor

@arizzi arizzi commented Apr 10, 2017

Add k0 and lambdas from generalV0Candidates producer.
V0s are converted to VertexCompositePtrCandidate and rekeyed to point to packedPFcandidate.
In addition tracks from V0s are white listed

Size increase is 150bytes/event on ttbar
http://arizzi.web.cern.ch/arizzi/miniaod/plusK0.html#recoVertexCompositePtrCandidates_slimmedKshortVertices__PAT

this PR is on top of most recent version of #18092

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

DataFormats/PatCandidates
PhysicsTools/PatAlgos

@perrotta, @cmsbuild, @slava77, @monttj, @davidlange6 can you please review it and eventually sign? Thanks.
@TaiSakuma, @gouskos, @JyothsnaKomaragiri, @imarches, @ahinzmann, @acaudron, @mmarionncern, @rappoccio, @jdolen, @nhanvtran, @gpetruc, @gkasieczka, @schoef, @ferencek, @mverzett, @mariadalfonso, @pvmulder, @cbernet this is something you requested to watch as well.
@Muzaffar, @davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 10, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19073/console Started: 2017/04/10 22:55

@cmsbuild
Copy link
Contributor

@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-18300/19073/summary.html

Comparison Summary:

  • You potentially added 7 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 3219 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1917209
  • DQMHistoTests: Total failures: 30977
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1886059
  • DQMHistoTests: Total skipped: 173
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 14 edm output root files, 23 DQM output files

@alberto-sanchez
Copy link
Member

@arizzi, working on that.

@arizzi
Copy link
Contributor Author

arizzi commented Apr 11, 2017

same update as #18092 to avoid root files in release

@cmsbuild
Copy link
Contributor

Pull request #18300 was updated. @perrotta, @cmsbuild, @slava77, @monttj, @davidlange6 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Apr 13, 2017

@arizzi
there are merge conflicts here.
Please rebase or remake&force-push

@arizzi
Copy link
Contributor Author

arizzi commented Apr 13, 2017

everything is in the last commit, I'll cherry pick and force update

@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-18300/19168/summary.html

Comparison Summary:

  • You potentially added 786 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 3267 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1818598
  • DQMHistoTests: Total failures: 40859
  • DQMHistoTests: Total nulls: 13
  • DQMHistoTests: Total successes: 1777553
  • DQMHistoTests: Total skipped: 173
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 14 edm output root files, 23 DQM output files

@slava77
Copy link
Contributor

slava77 commented Apr 14, 2017

@cmsbuild please test
to get cleaner comparisons now that CMSSW_9_1_X_2017-04-13-2300 is available (effective merge base for this PR)

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 14, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19183/console Started: 2017/04/14 15:15

@cmsbuild
Copy link
Contributor

@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-18300/19183/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 3187 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1818576
  • DQMHistoTests: Total failures: 30780
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 1787622
  • DQMHistoTests: Total skipped: 173
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 14 edm output root files, 23 DQM output files

@@ -32,6 +37,7 @@ namespace pat {
pat::PATSecondaryVertexSlimmer::PATSecondaryVertexSlimmer(const edm::ParameterSet& iConfig) :
src_(consumes<reco::VertexCompositePtrCandidateCollection>(iConfig.getParameter<edm::InputTag>("src"))),
srcLegacy_(mayConsume<std::vector<reco::Vertex> >(iConfig.getParameter<edm::InputTag>("src"))),
srcV0s_(mayConsume<reco::VertexCompositeCandidateCollection>(iConfig.getParameter<edm::InputTag>("src"))),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this runtime input type detection is getting only worse.
Please change the module configuration to pass the type to be used at configuration step.

@Dr15Jones is proliferation of this mayConsume to different types OK for the framework, or do we get some unwanted cost?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mayConsumes is presently (and may always) trigger prefetching of the item requested. It is possible to determine what type is available before calling consumes. This is done by registering a function to call when new products are registered. The registration is done by passing a functor to callWhenNewProductsRegistered. The documentation can be found here:
https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideEDMGetDataFromEvent#GetterOfProducts

@slava77
Copy link
Contributor

slava77 commented Apr 18, 2017

+1

for #18300 4b691fa

  • implemented as described
  • jenkins tests pass and comparisons show differences in packed candidate delta plots where more cands have filled values (more so in the /lostTrack folder) as expected
  • local test with PAT step running on 136.76 (2016E double muon) shows addition of slimmedKshortVertices and slimmedLambdaVertices modules which take under 0.1ms/event total and produce VertexCompositePtrCandidates collections with size consistent with what's in the PR description.

#18390 is created to possibly follow up on implementation of PhysicsTools/PatAlgos/plugins/PATSecondaryVertexSlimmer.cc

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

7 participants