Using new Event::get and ::getHandle in PFProducers #25681

guitargeek · 2019-01-16T16:51:07Z

This PR builds on top of #25707 to replace the occurrences of getByToken with the more modern get and getHandle in RecoParticleFlow/PFProducer.

cmsbuild · 2019-01-16T16:51:37Z

The code-checks are being triggered in jenkins.

guitargeek · 2019-01-16T16:54:23Z

RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc

  //Assign the PFAlgo Parameters
-  setPFVertexParameters(vertices.product());


I think if there are no vertices in the event, the call to Handle::product() will already raise a readable error [1], so the log might not be needed?

[1] https://github.com/cms-sw/cmssw/blob/master/FWCore/Framework/interface/GenericHandle.h#L105

cmsbuild · 2019-01-16T16:57:31Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25681/8031

This PR adds an extra 96KB to repository

guitargeek · 2019-01-16T16:57:37Z

RecoParticleFlow/PFProducer/plugins/PFEGammaProducer.cc

  LOGDRESSED("PFEGammaProducer")<<"getting blocks"<<std::endl;
-  bool found = iEvent.getByToken( inputTagBlocks_, blocks );  


This would be a good opportunity to use the auto handle = iEvent.getHandle(token_).makeValidHandle(); as @makortel suggested. Then, the following checks would truly unnecessary. Do you already have a specific implementation is mind for makeValidHandle()?

cmsbuild · 2019-01-16T16:57:58Z

A new Pull Request was created by @guitargeek (Jonas Rembser) for master.

It involves the following packages:

FWCore/Framework
RecoParticleFlow/PFProducer

@perrotta, @smuzaffar, @cmsbuild, @Dr15Jones, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @makortel, @cbernet, @wddgit, @lgray, @Martin-Grunewald, @seemasharmafnal, @hatakeyamak, @bachtis 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

guitargeek · 2019-01-16T16:59:36Z

RecoParticleFlow/PFProducer/plugins/PFProducer.cc


  //Assign the PFAlgo Parameters
-  pfAlgo_->setPFVertexParameters(useVerticesForNeutral_,vertices.product());


Same as for PFEGammaProducer; I think the logging before is not necessary because product() throws an exception already.

Dr15Jones · 2019-01-16T17:03:46Z

-1
@guitargeek I'm glad you are enthusiastic about our suggestions, however the framework group is still working out exactly how we want to do it. Once we have decided, we will make the needed changes to Event, Run and LuminosityBlock.

guitargeek · 2019-01-16T17:06:45Z

Ok! I'm happy to know you are working on it, that was not really clear in the issue thread! I suppose I can keep using this version privately and can then migrate my analysis code to what you will come up with without any effort :)

cmsbuild · 2019-01-20T23:51:37Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-01-20T23:57:34Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25681/8099

This PR adds an extra 88KB to repository

cmsbuild · 2019-01-24T06:56:44Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32808/console Started: 2019/01/24 07:58

cmsbuild · 2019-01-24T08:51:38Z

+1
Tested at: 73aa52a
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25681/32808/summary.html

cmsbuild · 2019-01-24T08:51:44Z

Comparison job queued.

cmsbuild · 2019-01-24T10:49:53Z

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

Comparison Summary:

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

perrotta · 2019-01-28T15:13:59Z

+1

It applies the new functionalities provided in Provide easier to use ::get and ::getHandle methods for Event, Run and LuminosityBlock #25707, as in the description of this PR
Jenkins tests pass and show no differences

cmsbuild · 2019-01-28T15:14:29Z

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)

fabiocos · 2019-01-29T12:31:43Z

code-checks

cmsbuild · 2019-01-29T12:32:11Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-01-29T12:38:20Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25681/8203

This PR adds an extra 92KB to repository

fabiocos · 2019-01-29T17:45:51Z

+1

cmsbuild added this to the CMSSW_10_5_X milestone Jan 16, 2019

cmsbuild added code-checks-pending comparison-pending core-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jan 16, 2019

guitargeek commented Jan 16, 2019

View reviewed changes

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 16, 2019

guitargeek commented Jan 16, 2019

View reviewed changes

cmsbuild added core-rejected and removed core-pending labels Jan 16, 2019

guitargeek closed this Jan 16, 2019

guitargeek reopened this Jan 20, 2019

Remove getByToken from RecoParticleFlow/PFProducer

73aa52a

cmsbuild added code-checks-pending and removed code-checks-approved core-rejected labels Jan 20, 2019

guitargeek changed the title ~~First step in modernizing product retrieval from Event~~ Using new Event::get and ::getHandle in PFProducers Jan 20, 2019

cmsbuild removed the code-checks-pending label Jan 20, 2019

cmsbuild added comparison-pending tests-pending labels Jan 24, 2019

cmsbuild added tests-started and removed tests-pending labels Jan 24, 2019

cmsbuild added tests-approved and removed tests-started labels Jan 24, 2019

cmsbuild added comparison-available and removed comparison-pending labels Jan 24, 2019

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Jan 28, 2019

cmsbuild added code-checks-pending and removed code-checks-approved labels Jan 29, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 29, 2019

cmsbuild added orp-approved and removed orp-pending labels Jan 29, 2019

cmsbuild merged commit 0530f78 into cms-sw:master Jan 29, 2019

guitargeek deleted the new_event_interface_pfproducers branch January 29, 2019 18:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using new Event::get and ::getHandle in PFProducers #25681

Using new Event::get and ::getHandle in PFProducers #25681

guitargeek commented Jan 16, 2019 •

edited

cmsbuild commented Jan 16, 2019

guitargeek Jan 16, 2019 •

edited

cmsbuild commented Jan 16, 2019

guitargeek Jan 16, 2019

cmsbuild commented Jan 16, 2019

guitargeek Jan 16, 2019

Dr15Jones commented Jan 16, 2019

guitargeek commented Jan 16, 2019

cmsbuild commented Jan 20, 2019

cmsbuild commented Jan 20, 2019

cmsbuild commented Jan 24, 2019 •

edited

cmsbuild commented Jan 24, 2019

cmsbuild commented Jan 24, 2019

cmsbuild commented Jan 24, 2019

perrotta commented Jan 28, 2019

cmsbuild commented Jan 28, 2019

fabiocos commented Jan 29, 2019

cmsbuild commented Jan 29, 2019

cmsbuild commented Jan 29, 2019

fabiocos commented Jan 29, 2019

		//Assign the PFAlgo Parameters
		setPFVertexParameters(vertices.product());

		LOGDRESSED("PFEGammaProducer")<<"getting blocks"<<std::endl;
		bool found = iEvent.getByToken( inputTagBlocks_, blocks );


		//Assign the PFAlgo Parameters
		pfAlgo_->setPFVertexParameters(useVerticesForNeutral_,vertices.product());

Using new Event::get and ::getHandle in PFProducers #25681

Using new Event::get and ::getHandle in PFProducers #25681

Conversation

guitargeek commented Jan 16, 2019 • edited

cmsbuild commented Jan 16, 2019

guitargeek Jan 16, 2019 • edited

Choose a reason for hiding this comment

cmsbuild commented Jan 16, 2019

guitargeek Jan 16, 2019

Choose a reason for hiding this comment

cmsbuild commented Jan 16, 2019

guitargeek Jan 16, 2019

Choose a reason for hiding this comment

Dr15Jones commented Jan 16, 2019

guitargeek commented Jan 16, 2019

cmsbuild commented Jan 20, 2019

cmsbuild commented Jan 20, 2019

cmsbuild commented Jan 24, 2019 • edited

cmsbuild commented Jan 24, 2019

cmsbuild commented Jan 24, 2019

cmsbuild commented Jan 24, 2019

perrotta commented Jan 28, 2019

cmsbuild commented Jan 28, 2019

fabiocos commented Jan 29, 2019

cmsbuild commented Jan 29, 2019

cmsbuild commented Jan 29, 2019

fabiocos commented Jan 29, 2019

guitargeek commented Jan 16, 2019 •

edited

guitargeek Jan 16, 2019 •

edited

cmsbuild commented Jan 24, 2019 •

edited