Layer Clustering for HGCal Scintillator #27485

jingyucms · 2019-07-11T00:33:36Z

PR description:

This PR modifies the CLUE algorithm for the scintillator section of the HGCal. It basically changes the distance calculation from x-y space to eta-phi space for the scintillator. This is motivated by the fact that the scintillator cells are uniform in eta-phi rather than x-y. The implementation has been presented in the following HGCal DPG meetings:

@rovere @felicepantaleo

PR validation:

The PR was validated locally and passes:
scram build code-checks
scram b runtests
runTheMatrix.py -l limited -i all --ibeos

if this PR is a backport please specify the original PR: No.

Before submitting your pull requests, make sure you followed this checklist:

verify that the PR is really intended for the chosen branch
verify that changes follow CMS Naming, Coding, And Style Rules
verify that the PR passes the basic test procedure suggested in the CMSSW PR instructions

cmsbuild · 2019-07-11T00:34:08Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-07-11T00:41:24Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27485/10819

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

cmsbuild · 2019-07-11T00:51:13Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-07-11T00:59:00Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27485/10820

This PR adds an extra 84KB to repository

cmsbuild · 2019-08-01T17:05:27Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-08-01T17:11:46Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27485/11246

This PR adds an extra 96KB to repository

cmsbuild · 2019-08-01T17:12:14Z

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

kpedro88 · 2019-08-01T18:02:56Z

please test

cmsbuild · 2019-08-01T18:03:34Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1781/console Started: 2019/08/01 20:04

cmsbuild · 2019-08-01T19:54:47Z

+1
Tested at: 223cb8c
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d2eb64/1781/summary.html

cmsbuild · 2019-08-01T19:54:51Z

Comparison job queued.

cmsbuild · 2019-08-01T21:52:50Z

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

Comparison Summary:

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

slava77 · 2019-08-01T23:04:14Z

+1

for #27485 223cb8c

code changes are in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show differences starting from the hgcalLayerClusters, e.g. Layer Clustering for HGCal Scintillator #27485 (comment). The limited number of events used in the tests only shows that changes are roughly in the right places.
- for the physics quality checks I'm deferring to the work done in the DPG and Layer Clustering for HGCal Scintillator #27485 (comment)

kpedro88

A few followup comments after the changes from the reco review. These will not affect the results of the PR in any way, so they can be implemented in a followup PR.

kpedro88 · 2019-08-02T15:49:30Z

RecoLocalCalo/HGCalRecAlgos/src/RecHitTools.cc

@@ -409,6 +410,18 @@ bool RecHitTools::isHalfCell(const DetId& id) const {
  return ishalf;
 }

+bool RecHitTools::isSilicon(const DetId& id) const {
+  bool issilicon = false;


this can all be simplified to:

return (id.det() == DetId::HGCalEE || id.det() == DetId::HGCalHSi);

kpedro88 · 2019-08-02T15:50:04Z

RecoLocalCalo/HGCalRecAlgos/src/RecHitTools.cc

+
+bool RecHitTools::isOnlySilicon(const unsigned int layer) const {
+  bool isonlysilicon = (layer % bhLastLayer_) < bhOffset_;
+  return isonlysilicon;


simplify:

return (layer % bhLastLayer_) < bhOffset_;

kpedro88 · 2019-08-02T15:51:25Z

RecoLocalCalo/HGCalRecProducers/interface/HGCalLayerTiles.h

  int getGlobalBin(float x, float y) const { return getXBin(x) + getYBin(y) * hgcaltilesconstants::nColumns; }

  int getGlobalBinByBin(int xBin, int yBin) const { return xBin + yBin * hgcaltilesconstants::nColumns; }

+  int getGlobalBinEtaPhi(float eta, float phi) const {
+    return hgcaltilesconstants::nColumns * hgcaltilesconstants::nRows + getEtaBin(eta) +


to reduce duplication slightly:

return getGlobalBinByBinEtaPhi(getEtaBin(eta),getPhiBin(phi));

kpedro88 · 2019-08-02T15:52:34Z

RecoLocalCalo/HGCalRecProducers/plugins/HGCalCLUEAlgo.cc

  auto& cellsOnLayer = cells_[layerId];
  unsigned int numberOfCells = cellsOnLayer.detid.size();
+  bool isOnlySi(false);


simplify:

bool isOnlySi(rhtools_.isOnlySilicon(layerId));

kpedro88 · 2019-08-02T15:53:22Z

RecoLocalCalo/HGCalRecProducers/plugins/HGCalCLUEAlgo.cc

+          for (unsigned int j = 0; j < binSize; j++) {
+            unsigned int otherId = lt[binId][j];
+            bool otherSi = isOnlySi || cellsOnLayer.isSi[otherId];
+            if (otherSi) {  //silicon cells cannot talk to scintillator cells


simplify:

if (otherSi and distance(i, otherId, layerId, false) < delta) {

and put the comment on the previous line

kpedro88 · 2019-08-02T15:53:54Z

RecoLocalCalo/HGCalRecProducers/plugins/HGCalCLUEAlgo.cc

+
+          for (unsigned int j = 0; j < binSize; j++) {
+            unsigned int otherId = lt[binId][j];
+            if (!cellsOnLayer.isSi[otherId]) {  //scintillator cells cannot talk to silicon cells


simplify:

if (!cellsOnLayer.isSi[otherId] and distance(i, otherId, layerId, true) < delta) {

and put the comment on the previous line

kpedro88 · 2019-08-02T15:55:04Z

RecoLocalCalo/HGCalRecProducers/plugins/HGCalCLUEAlgo.cc

  auto& cellsOnLayer = cells_[layerId];
  unsigned int numberOfCells = cellsOnLayer.detid.size();
+  bool isOnlySi(false);


simplify:

bool isOnlySi(rhtools_.isOnlySilicon(layerId));

kpedro88 · 2019-08-02T15:56:24Z

+upgrade
a few minor simplifications can be addressed in a followup PR

cmsbuild · 2019-08-02T15:56:53Z

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)

kpedro88 · 2019-08-02T21:40:06Z

+1

jingyucms added 12 commits June 13, 2019 08:28

first commit for layer cluster in scintillator

8fbb42e

first commit for layer cluster in scintillator

a2918b6

fix a bug + some cleanup

c915450

some cleanup

75ea77e

some cleanup

aad7864

first commit for layer cluster in scintillator

959ce63

fix a bug + some cleanup

09e12d8

avoid multiple seeds when adjacent cells have the same density

e0db8ff

add 3x3 window scenario for density calculation

04e9708

fix a typo

2ef7dad

solve merge conflict

789a5c3

some additional cleanup

7725e0f

cmsbuild added this to the CMSSW_11_0_X milestone Jul 11, 2019

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending upgrade-pending labels Jul 11, 2019

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 11, 2019

code formatting

8fcff15

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jul 11, 2019

cmsbuild removed the code-checks-pending label Jul 11, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 1, 2019

cmsbuild added tests-started and removed tests-pending labels Aug 1, 2019

cmsbuild added tests-approved and removed tests-started labels Aug 1, 2019

cmsbuild added comparison-available and removed comparison-pending labels Aug 1, 2019

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Aug 1, 2019

kpedro88 reviewed Aug 2, 2019

View reviewed changes

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Aug 2, 2019

cmsbuild added orp-approved and removed orp-pending labels Aug 2, 2019

cmsbuild merged commit d50e504 into cms-sw:master Aug 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Layer Clustering for HGCal Scintillator #27485

Layer Clustering for HGCal Scintillator #27485

jingyucms commented Jul 11, 2019 •

edited

cmsbuild commented Jul 11, 2019

cmsbuild commented Jul 11, 2019

cmsbuild commented Jul 11, 2019

cmsbuild commented Jul 11, 2019

cmsbuild commented Aug 1, 2019

cmsbuild commented Aug 1, 2019

cmsbuild commented Aug 1, 2019

kpedro88 commented Aug 1, 2019

cmsbuild commented Aug 1, 2019 •

edited

cmsbuild commented Aug 1, 2019

cmsbuild commented Aug 1, 2019

cmsbuild commented Aug 1, 2019

slava77 commented Aug 1, 2019

kpedro88 left a comment

kpedro88 Aug 2, 2019

kpedro88 Aug 2, 2019

kpedro88 Aug 2, 2019

kpedro88 Aug 2, 2019

kpedro88 Aug 2, 2019

kpedro88 Aug 2, 2019

kpedro88 Aug 2, 2019

kpedro88 commented Aug 2, 2019

cmsbuild commented Aug 2, 2019

kpedro88 commented Aug 2, 2019

Layer Clustering for HGCal Scintillator #27485

Layer Clustering for HGCal Scintillator #27485

Conversation

jingyucms commented Jul 11, 2019 • edited

PR description:

PR validation:

if this PR is a backport please specify the original PR: No.

cmsbuild commented Jul 11, 2019

cmsbuild commented Jul 11, 2019

cmsbuild commented Jul 11, 2019

cmsbuild commented Jul 11, 2019

cmsbuild commented Aug 1, 2019

cmsbuild commented Aug 1, 2019

cmsbuild commented Aug 1, 2019

kpedro88 commented Aug 1, 2019

cmsbuild commented Aug 1, 2019 • edited

cmsbuild commented Aug 1, 2019

cmsbuild commented Aug 1, 2019

cmsbuild commented Aug 1, 2019

slava77 commented Aug 1, 2019

kpedro88 left a comment

Choose a reason for hiding this comment

kpedro88 Aug 2, 2019

Choose a reason for hiding this comment

kpedro88 Aug 2, 2019

Choose a reason for hiding this comment

kpedro88 Aug 2, 2019

Choose a reason for hiding this comment

kpedro88 Aug 2, 2019

Choose a reason for hiding this comment

kpedro88 Aug 2, 2019

Choose a reason for hiding this comment

kpedro88 Aug 2, 2019

Choose a reason for hiding this comment

kpedro88 Aug 2, 2019

Choose a reason for hiding this comment

kpedro88 commented Aug 2, 2019

cmsbuild commented Aug 2, 2019

kpedro88 commented Aug 2, 2019

jingyucms commented Jul 11, 2019 •

edited

cmsbuild commented Aug 1, 2019 •

edited