Migrate RecoParticleFlow/PFTracking to esConsumes #33367

marksan87 · 2021-04-08T03:37:39Z

PR description:

Migration of RecoParticleFlow/PFTracking package to esConsumes to address #31061

PR validation:

Run standard limited matrix evaluation

runTheMatrix.py -l limited -j 8 --ibeos

if this PR is a backport please specify the original PR and why you need to backport that PR:

Not a backport

@hatakeyamak

cmsbuild · 2021-04-08T03:45:27Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33367/21956

This PR adds an extra 48KB to repository

cmsbuild · 2021-04-08T03:45:50Z

A new Pull Request was created by @marksan87 (Mark Saunders) for master.

It involves the following packages:

RecoParticleFlow/PFTracking

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @makortel, @cbernet, @rovere, @lgray, @lecriste, @hatakeyamak, @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 · 2021-04-08T04:51:10Z

@cmsbuild please test

hatakeyamak · 2021-04-08T12:42:09Z

@bendavid @laurenhay @rappoccio

jpata · 2021-04-09T06:21:19Z

@cmsbuild please test

I can't find the previous test in jenkins, seems stuck/lost.

cmsbuild · 2021-04-09T09:48:00Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-766e47/14124/summary.html
COMMIT: 86466c8
CMSSW: CMSSW_11_3_X_2021-04-08-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33367/14124/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS Clang-Tidy warnings: There are Clang-Tidy warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-766e47/14124/llvm-analysis/cmsclangtidy.txt for details.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2865506
DQMHistoTests: Total failures: 12
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2865471
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 37 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

jpata · 2021-04-09T13:53:48Z

No unexpected reco changes detected. Code changes look good, a minor comment inline.

Looks like there's another get to migrate in GoodSeedProducer. You can get this info from the bot Comparison Summary. Summary -> CMS clang-tidy checks EventSetupRecord::get warnings

/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-04-08-2300/src/RecoParticleFlow/PFTracking/plugins/GoodSeedProducer.cc:620:3: warning: direct call of function EventSetupRecord::get(ESHandle&) is deprecated and should be replaced with a call to EventSetup::getHandle(ESGetToken&). To use ESGetToken see https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideHowToGetDataFromES#In_ED_module To get data with the token see https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideHowToGetDataFromES#Getting_data_from_EventSetup_wit [cms-esrget]

jpata · 2021-04-09T13:46:48Z

RecoParticleFlow/PFTracking/plugins/HGCalTrackCollectionProducer.cc

  LogDebug("HGCalTrackCollectionProducer")
      << " HGCalTrackCollectionProducer::HGCalTrackCollectionProducer " << std::endl;

  const edm::ParameterSet& geoconf = iConfig.getParameterSet("hgcalGeometryNames");
  hgc_names_[0] = geoconf.getParameter<std::string>("HGC_ECAL");
+  for (unsigned i = 0; i < hgcGeometryTokens_.size(); i++) {


Suggested change

for (unsigned i = 0; i < hgcGeometryTokens_.size(); i++) {

for (const auto i : hgcGeometryTokens_) {

In fact, I'm a little confused why you need an std::array of length 1 here.

This format was kept to allow for future extrapolation to HGCal hadronic sections.

cmsbuild · 2021-04-09T14:45:01Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33367/21995

This PR adds an extra 20KB to repository

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

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33367/21995/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33367/21995/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-04-09T15:02:43Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33367/21998

This PR adds an extra 28KB to repository

cmsbuild · 2021-04-09T15:03:07Z

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

jpata · 2021-04-12T10:25:35Z

@cmsbuild please test

cmsbuild · 2021-04-12T13:11:34Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-766e47/14173/summary.html
COMMIT: d61df15
CMSSW: CMSSW_11_3_X_2021-04-11-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33367/14173/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

No significant changes to the logs found
Reco comparison results: 1264 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2865506
DQMHistoTests: Total failures: 3674
DQMHistoTests: Total nulls: 19
DQMHistoTests: Total successes: 2861791
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 45.703 KiB( 37 files compared)
DQMHistoSizes: changed ( 140.53 ): 44.531 KiB Hcal/DigiRunHarvesting
DQMHistoSizes: changed ( 140.53 ): 1.172 KiB RPC/DCSInfo
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

jpata · 2021-04-12T13:59:11Z

+reconstruction

technical esConsumes migration, no observed/expected reco changes (140.53 seems unrelated)

Thanks for this!

cmsbuild · 2021-04-12T13:59:35Z

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)

silviodonato · 2021-04-12T15:06:15Z

+1

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

hatakeyamak and others added 4 commits March 30, 2021 12:02

Use esComsumes.

a7e5549

Migrate PFTracking to esConsumes

f0ce0e4

Migrate PFTracking to esConsumes

6b9f1ac

Migrate PFTracking to esConsumes

86466c8

cmsbuild added this to the CMSSW_11_3_X milestone Apr 8, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 8, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 8, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 8, 2021

slava77 mentioned this pull request Apr 8, 2021

Finalising esConsumes migration of HLTrigger/ #33362

Merged

cmsbuild added tests-approved and removed tests-started labels Apr 9, 2021

jpata reviewed Apr 9, 2021

View reviewed changes

Migrate PFTracking to esConsumes

8b24a73

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-approved labels Apr 9, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Apr 9, 2021

Migrate PFTracking to esConsumes

d61df15

cmsbuild added code-checks-pending and removed code-checks-rejected labels Apr 9, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 9, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 12, 2021

cmsbuild added tests-approved and removed tests-started labels Apr 12, 2021

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Apr 12, 2021

cmsbuild added orp-approved and removed orp-pending labels Apr 12, 2021

cmsbuild merged commit 807000a into cms-sw:master Apr 12, 2021

cmsbuild mentioned this pull request Apr 12, 2021

Make the the size of the binner (HistoContainer) settable at run time. The total number of Pixel Clusters is not a limit anymore on GPU #33371

Merged

marksan87 deleted the PFesConsumesMigration branch April 27, 2021 00:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate RecoParticleFlow/PFTracking to esConsumes #33367

Migrate RecoParticleFlow/PFTracking to esConsumes #33367

marksan87 commented Apr 8, 2021

cmsbuild commented Apr 8, 2021

cmsbuild commented Apr 8, 2021

slava77 commented Apr 8, 2021

hatakeyamak commented Apr 8, 2021

jpata commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

jpata commented Apr 9, 2021

jpata Apr 9, 2021

jpata Apr 9, 2021

marksan87 Apr 9, 2021

cmsbuild commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

jpata commented Apr 12, 2021

cmsbuild commented Apr 12, 2021

jpata commented Apr 12, 2021

cmsbuild commented Apr 12, 2021

silviodonato commented Apr 12, 2021

	for (unsigned i = 0; i < hgcGeometryTokens_.size(); i++) {
	for (const auto i : hgcGeometryTokens_) {

Migrate RecoParticleFlow/PFTracking to esConsumes #33367

Migrate RecoParticleFlow/PFTracking to esConsumes #33367

Conversation

marksan87 commented Apr 8, 2021

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Apr 8, 2021

cmsbuild commented Apr 8, 2021

slava77 commented Apr 8, 2021

hatakeyamak commented Apr 8, 2021

jpata commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

Comparison Summary

jpata commented Apr 9, 2021

jpata Apr 9, 2021

Choose a reason for hiding this comment

jpata Apr 9, 2021

Choose a reason for hiding this comment

marksan87 Apr 9, 2021

Choose a reason for hiding this comment

cmsbuild commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

jpata commented Apr 12, 2021

cmsbuild commented Apr 12, 2021

Comparison Summary

jpata commented Apr 12, 2021

cmsbuild commented Apr 12, 2021

silviodonato commented Apr 12, 2021