Phase2-hgx294Z Modify the getCorner algo of HGCalGeometry to take care of rotated layers #35998

bsunanda · 2021-11-04T21:43:58Z

PR description:

Modify the getCorner algo of HGCalGeometry to take care of rotated layers

PR validation:

Use cfg's in Geometry/HGCalGeometry/test area as we'll as runTheMatrix test workflows

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

Nothing special

…yers

cmsbuild · 2021-11-04T21:49:20Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35998/26429

This PR adds an extra 36KB 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-35998/26429/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35998/26429/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-11-04T22:01:22Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35998/26431

This PR adds an extra 40KB to repository

cmsbuild · 2021-11-04T22:01:43Z

A new Pull Request was created by @bsunanda (Sunanda Banerjee) for master.

It involves the following packages:

Geometry/HGCalCommonData (geometry, upgrade)
Geometry/HGCalGeometry (geometry, upgrade)

@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please review it and eventually sign? Thanks.
@fabiocos this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

bsunanda · 2021-11-04T22:07:15Z

@cmsbuild Please test

cmsbuild · 2021-11-05T03:32:55Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cd28e3/20263/summary.html
COMMIT: e99793f
CMSSW: CMSSW_12_2_X_2021-11-04-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35998/20263/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 2901890
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2901868
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

cvuosalo · 2021-11-05T19:25:47Z

+1

bsunanda · 2021-11-05T20:20:45Z

@AdrianoDee, @srimanob please review and sign

srimanob · 2021-11-06T02:27:21Z

Geometry/HGCalCommonData/src/HGCalDDDConstants.cc

+  bool rotx =
+      ((!hgpar_->layerType_.empty()) && (hgpar_->layerType_[lay - hgpar_->firstLayer_] == HGCalTypes::WaferCenterR));
+#ifdef EDM_ML_DEBUG
+  if (debug)


Why do you need this if under ifdef?

srimanob · 2021-11-06T02:27:35Z

Geometry/HGCalCommonData/src/HGCalDDDConstants.cc

+  x += xy.first;
+  y += xy.second;
+#ifdef EDM_ML_DEBUG
+  if (debug)


cmsbuild · 2021-11-06T13:46:26Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35998/26471

This PR adds an extra 40KB to repository

cmsbuild · 2021-11-06T13:46:48Z

Pull request #35998 was updated. @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please check and sign again.

srimanob · 2021-11-06T14:00:53Z

@cmsbuild please test

cmsbuild · 2021-11-06T17:20:09Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cd28e3/20299/summary.html
COMMIT: 3c3e060
CMSSW: CMSSW_12_2_X_2021-11-06-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35998/20299/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 2901890
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2901868
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

srimanob · 2021-11-06T17:29:09Z

+Upgrade

civanch · 2021-11-06T18:17:06Z

+1

was approved by geometry before

cmsbuild · 2021-11-06T18:17:31Z

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

qliphy · 2021-11-07T01:50:37Z

+1

Modify the getCorner algo of HGCalGeometry to take care of rotated la…

76e27b0

…yers

cmsbuild added this to the CMSSW_12_2_X milestone Nov 4, 2021

cmsbuild added code-checks-pending geometry-pending orp-pending pending-signatures tests-pending upgrade-pending labels Nov 4, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Nov 4, 2021

Code check

e99793f

cmsbuild added code-checks-pending and removed code-checks-rejected labels Nov 4, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 4, 2021

cmsbuild added tests-started and removed tests-pending labels Nov 4, 2021

cmsbuild added tests-approved and removed tests-started labels Nov 5, 2021

cmsbuild added geometry-approved and removed geometry-pending labels Nov 5, 2021

srimanob reviewed Nov 6, 2021

View reviewed changes

Geometry/HGCalCommonData/src/HGCalDDDConstants.cc

x += xy.first;

y += xy.second;

#ifdef EDM_ML_DEBUG

if (debug)

Copy link

Contributor

srimanob Nov 6, 2021

Choose a reason for hiding this comment

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

Same here.

cmsbuild added code-checks-pending geometry-pending tests-pending and removed geometry-approved tests-approved code-checks-approved labels Nov 6, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 6, 2021

cmsbuild added tests-started and removed tests-pending labels Nov 6, 2021

cmsbuild added tests-approved and removed tests-started labels Nov 6, 2021

cmsbuild added upgrade-approved and removed upgrade-pending labels Nov 6, 2021

cmsbuild added fully-signed geometry-approved and removed geometry-pending pending-signatures labels Nov 6, 2021

cmsbuild added orp-approved and removed orp-pending labels Nov 7, 2021

cmsbuild merged commit 841bcc3 into cms-sw:master Nov 7, 2021

cmsbuild mentioned this pull request Nov 8, 2021

rootmodule: Updated root to tip of branch master cms-sw/cmsdist#7431

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Phase2-hgx294Z Modify the getCorner algo of HGCalGeometry to take care of rotated layers #35998

Phase2-hgx294Z Modify the getCorner algo of HGCalGeometry to take care of rotated layers #35998

bsunanda commented Nov 4, 2021

cmsbuild commented Nov 4, 2021

cmsbuild commented Nov 4, 2021

cmsbuild commented Nov 4, 2021

bsunanda commented Nov 4, 2021

cmsbuild commented Nov 5, 2021

cvuosalo commented Nov 5, 2021

bsunanda commented Nov 5, 2021

srimanob Nov 6, 2021

srimanob Nov 6, 2021

cmsbuild commented Nov 6, 2021

cmsbuild commented Nov 6, 2021

srimanob commented Nov 6, 2021

cmsbuild commented Nov 6, 2021

srimanob commented Nov 6, 2021

civanch commented Nov 6, 2021

cmsbuild commented Nov 6, 2021

qliphy commented Nov 7, 2021

Phase2-hgx294Z Modify the getCorner algo of HGCalGeometry to take care of rotated layers #35998

Phase2-hgx294Z Modify the getCorner algo of HGCalGeometry to take care of rotated layers #35998

Conversation

bsunanda commented Nov 4, 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 Nov 4, 2021

cmsbuild commented Nov 4, 2021

cmsbuild commented Nov 4, 2021

bsunanda commented Nov 4, 2021

cmsbuild commented Nov 5, 2021

Comparison Summary

cvuosalo commented Nov 5, 2021

bsunanda commented Nov 5, 2021

srimanob Nov 6, 2021

Choose a reason for hiding this comment

srimanob Nov 6, 2021

Choose a reason for hiding this comment

cmsbuild commented Nov 6, 2021

cmsbuild commented Nov 6, 2021

srimanob commented Nov 6, 2021

cmsbuild commented Nov 6, 2021

Comparison Summary

srimanob commented Nov 6, 2021

civanch commented Nov 6, 2021

cmsbuild commented Nov 6, 2021

qliphy commented Nov 7, 2021