added esConsumes to modules on EventFilter/SiPixelRawToDigi #30474

JamminJones · 2020-06-30T16:31:33Z

PR description:

added esConsumes to 3 modules

PR validation:

the code compiles

cmsbuild · 2020-06-30T16:32:01Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-06-30T16:39:12Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30474/16635

This PR adds an extra 24KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File EventFilter/SiPixelRawToDigi/plugins/SiPixelDigiToRaw.cc modified in PR(s): [RFC] Test clang-tidy --checks modernize-use-auto #29936
- File EventFilter/SiPixelRawToDigi/plugins/SiPixelRawToDigi.cc modified in PR(s): [RFC] Test clang-tidy --checks modernize-make-unique #29934, [RFC] Test clang-tidy --checks modernize-loop-convert #29935, [RFC] Test clang-tidy --checks modernize-use-auto #29936
- File EventFilter/SiPixelRawToDigi/src/PixelUnpackingRegions.cc modified in PR(s): [RFC] Test clang-tidy --checks modernize-loop-convert #29935

cmsbuild · 2020-06-30T16:39:37Z

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

It involves the following packages:

EventFilter/SiPixelRawToDigi

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @Martin-Grunewald, @dkotlins, @VinInn this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2020-06-30T17:14:11Z

This change was made under my supervision.

Dr15Jones · 2020-06-30T17:16:20Z

please test

cmsbuild · 2020-06-30T17:16:43Z

The tests are being triggered in jenkins.

CMSSW_11_2_X_2020-06-30-1100/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/7540/console Started: 2020/06/30 19:17

cmsbuild · 2020-06-30T18:29:55Z

+1
Tested at: 1556c63
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cb02dc/7540/summary.html
CMSSW: CMSSW_11_2_X_2020-06-30-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-06-30T18:29:58Z

Comparison job queued.

cmsbuild · 2020-06-30T20:16:48Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 36
DQMHistoTests: Total histograms compared: 2778915
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2778858
DQMHistoTests: Total skipped: 50
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
Checked 152 log files, 16 edm output root files, 36 DQM output files

slava77

static analyzer pointed out remaining es.get calls in files modified by this PR.
Is there a reason to skip these?

slava77 · 2020-06-30T21:41:22Z

EventFilter/SiPixelRawToDigi/src/PixelUnpackingRegions.cc

@@ -81,9 +80,8 @@ void PixelUnpackingRegions::initialize(const edm::EventSetup& es) {
    es.get<SiPixelFedCablingMapRcd>().get(cablingMap);


(from the static analyzer)
shouldn't this be in esConsumes/token as well?

Yes. I think this is the first time we've encountered an ESTransientHandle being used. Looking at the code, though, this should NOT be an ESTransientHandle since it is holding onto memory from the data product.

slava77 · 2020-06-30T21:43:18Z

EventFilter/SiPixelRawToDigi/plugins/SiPixelRawToDigi.cc

@@ -161,8 +162,7 @@ void SiPixelRawToDigi::produce(edm::Event& ev, const edm::EventSetup& es) {
  // initialize quality record or update if necessary


should the es.get<SiPixelFedCablingMapRcd>().get(cablingMapLabel, cablingMap); be addressed as well?

cmsbuild · 2020-07-01T15:16:27Z

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

Dr15Jones · 2020-07-01T15:30:43Z

EventFilter/SiPixelRawToDigi/plugins/SiPixelRawToDigi.cc

@@ -152,17 +153,15 @@ void SiPixelRawToDigi::produce(edm::Event& ev, const edm::EventSetup& es) {
  // initialize cabling map or update if necessary
  if (recordWatcher.check(es)) {
    // cabling map, which maps online address (fed->link->ROC->local pixel) to offline (DetId->global pixel)
-    edm::ESTransientHandle<SiPixelFedCablingMap> cablingMap;
-    es.get<SiPixelFedCablingMapRcd>().get(cablingMapLabel, cablingMap);  //Tav
+    edm::ESHandle<SiPixelFedCablingMap> cablingMap = es.getHandle(tCablingMap);


NOTE: The previous use of ESTransientHandle here was inappropriate as the data product must not be allowed to be deleted before the end of the IOV since internal parts of the data product are being held.

Dr15Jones · 2020-07-01T15:32:00Z

EventFilter/SiPixelRawToDigi/src/PixelUnpackingRegions.cc

@@ -77,13 +77,11 @@ void PixelUnpackingRegions::initialize(const edm::EventSetup& es) {
  // initialize cabling map or update it if necessary
  // and re-cache modules information
  if (watcherSiPixelFedCablingMap_.check(es)) {
-    edm::ESTransientHandle<SiPixelFedCablingMap> cablingMap;
-    es.get<SiPixelFedCablingMapRcd>().get(cablingMap);
+    edm::ESHandle<SiPixelFedCablingMap> cablingMap = es.getHandle(cablingMapToken_);


NOTE: The previous use of ESTransientHandle is inappropriate here as the data product can not be allowed to be deleted before the end of the IOV as this module holds onto parts of the data product.

slava77 · 2020-07-01T20:03:19Z

@Dr15Jones
I'm not sure if your last comments were clarifications or if they implied more work.
If just clarifications, feel free to restart the tests

Dr15Jones · 2020-07-01T20:24:24Z

@slava77 they were a clarification, sorry about the confusion.

Dr15Jones · 2020-07-01T20:24:31Z

please test

cmsbuild · 2020-07-01T20:24:45Z

The tests are being triggered in jenkins.

CMSSW_11_2_X_2020-06-30-2300/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/7574/console Started: 2020/07/01 22:25

cmsbuild · 2020-07-01T21:39:55Z

+1
Tested at: 7033035
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cb02dc/7574/summary.html
CMSSW: CMSSW_11_2_X_2020-06-30-2300
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-07-01T21:39:58Z

Comparison job queued.

cmsbuild · 2020-07-01T23:32:52Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2784120
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2784063
DQMHistoTests: Total skipped: 50
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
Checked 154 log files, 17 edm output root files, 37 DQM output files

slava77 · 2020-07-01T23:45:52Z

+1

for #30474 7033035

essentially technical code changes are in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show no (relevant) differences

cmsbuild · 2020-07-01T23:46:15Z

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

slava77 · 2020-07-01T23:47:23Z

webhooks or the bot's reaction to them seems degraded. The "Comparison is ready" was posted a while back (more than 10 mins), but the label is still not updated.

silviodonato · 2020-07-02T08:35:11Z

+1

added esConsumes to modules on EventFilter/SiPixelRawToDigi

1556c63

cmsbuild added this to the CMSSW_11_2_X milestone Jun 30, 2020

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jun 30, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 30, 2020

cmsbuild added tests-started and removed tests-pending labels Jun 30, 2020

cmsbuild added tests-approved and removed tests-started labels Jun 30, 2020

cmsbuild added comparison-available and removed comparison-pending labels Jun 30, 2020

slava77 reviewed Jun 30, 2020

View reviewed changes

fixed remaining es.get calls

7033035

cmsbuild removed code-checks-approved comparison-available tests-approved labels Jul 1, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 1, 2020

Dr15Jones reviewed Jul 1, 2020

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Jul 1, 2020

cmsbuild added tests-approved and removed tests-started labels Jul 1, 2020

cmsbuild added comparison-available fully-signed reconstruction-approved and removed comparison-pending pending-signatures reconstruction-pending labels Jul 1, 2020

cmsbuild added orp-approved and removed orp-pending labels Jul 2, 2020

cmsbuild merged commit 29c804e into cms-sw:master Jul 2, 2020

cmsbuild mentioned this pull request Jul 2, 2020

[DataFormats/PatCandidates] Rework statics in header file. #30499

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added esConsumes to modules on EventFilter/SiPixelRawToDigi #30474

added esConsumes to modules on EventFilter/SiPixelRawToDigi #30474

JamminJones commented Jun 30, 2020

cmsbuild commented Jun 30, 2020

cmsbuild commented Jun 30, 2020

cmsbuild commented Jun 30, 2020

Dr15Jones commented Jun 30, 2020

Dr15Jones commented Jun 30, 2020

cmsbuild commented Jun 30, 2020 •

edited

cmsbuild commented Jun 30, 2020

cmsbuild commented Jun 30, 2020

cmsbuild commented Jun 30, 2020

slava77 left a comment

slava77 Jun 30, 2020

Dr15Jones Jul 1, 2020

slava77 Jun 30, 2020

cmsbuild commented Jul 1, 2020

Dr15Jones Jul 1, 2020 •

edited

Dr15Jones Jul 1, 2020

slava77 commented Jul 1, 2020

Dr15Jones commented Jul 1, 2020

Dr15Jones commented Jul 1, 2020

cmsbuild commented Jul 1, 2020 •

edited

cmsbuild commented Jul 1, 2020

cmsbuild commented Jul 1, 2020

cmsbuild commented Jul 1, 2020

slava77 commented Jul 1, 2020 •

edited

cmsbuild commented Jul 1, 2020

slava77 commented Jul 1, 2020

silviodonato commented Jul 2, 2020

		@@ -81,9 +80,8 @@ void PixelUnpackingRegions::initialize(const edm::EventSetup& es) {
		es.get<SiPixelFedCablingMapRcd>().get(cablingMap);

		@@ -161,8 +162,7 @@ void SiPixelRawToDigi::produce(edm::Event& ev, const edm::EventSetup& es) {
		// initialize quality record or update if necessary

added esConsumes to modules on EventFilter/SiPixelRawToDigi #30474

added esConsumes to modules on EventFilter/SiPixelRawToDigi #30474

Conversation

JamminJones commented Jun 30, 2020

PR description:

PR validation:

cmsbuild commented Jun 30, 2020

cmsbuild commented Jun 30, 2020

cmsbuild commented Jun 30, 2020

Dr15Jones commented Jun 30, 2020

Dr15Jones commented Jun 30, 2020

cmsbuild commented Jun 30, 2020 • edited

cmsbuild commented Jun 30, 2020

cmsbuild commented Jun 30, 2020

cmsbuild commented Jun 30, 2020

slava77 left a comment

Choose a reason for hiding this comment

slava77 Jun 30, 2020

Choose a reason for hiding this comment

Dr15Jones Jul 1, 2020

Choose a reason for hiding this comment

slava77 Jun 30, 2020

Choose a reason for hiding this comment

cmsbuild commented Jul 1, 2020

Dr15Jones Jul 1, 2020 • edited

Choose a reason for hiding this comment

Dr15Jones Jul 1, 2020

Choose a reason for hiding this comment

slava77 commented Jul 1, 2020

Dr15Jones commented Jul 1, 2020

Dr15Jones commented Jul 1, 2020

cmsbuild commented Jul 1, 2020 • edited

cmsbuild commented Jul 1, 2020

cmsbuild commented Jul 1, 2020

cmsbuild commented Jul 1, 2020

slava77 commented Jul 1, 2020 • edited

cmsbuild commented Jul 1, 2020

slava77 commented Jul 1, 2020

silviodonato commented Jul 2, 2020

cmsbuild commented Jun 30, 2020 •

edited

Dr15Jones Jul 1, 2020 •

edited

cmsbuild commented Jul 1, 2020 •

edited

slava77 commented Jul 1, 2020 •

edited