improved position measurement for 2D clusters in HGCAL silicon sensors #25411

amartelli · 2018-12-04T10:01:49Z

Improved position for 2D clusters, with focus on electromagnetic showers thus concerning 2Dcl centered in the Silicon sensors.
Main changes:

restrict the hits used to compute the position to those in a given radius around the most energetic hit (1.3cm)
adopt a log weighting of the energy, that is best suited for exponential shower profiles (with threshold at W0 = 2.9)
ignore difference between halo and border hits, just use them all
In support of this, see slides attached to the DPG agenda
https://indico.cern.ch/event/776790/contributions/3230583/attachments/1761511/2865143/positionResolutionFor2DClusters_amartell_29Nov.pdf

Further position optimisation will follow a global restyle with optimised parameters of the Imaging algorithm. In the specific for the position:

same optimisation for Scintillator part
same optimisation for "sharing" option enabled

cmsbuild · 2018-12-04T10:02:10Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-12-04T10:04:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25411/7453

cmsbuild · 2018-12-04T10:04:54Z

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

It involves the following packages:

RecoLocalCalo/HGCalRecAlgos
RecoLocalCalo/HGCalRecProducers

@perrotta, @cmsbuild, @kpedro88, @slava77 can you please review it and eventually sign? Thanks.
@edjtscott, @vandreev11, @sethzenz, @felicepantaleo, @rovere, @argiro, @cseez, @pfs, @lgray, @kpedro88 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

perrotta · 2018-12-04T13:42:56Z

please test

cmsbuild · 2018-12-04T13:43:12Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/31980/console Started: 2018/12/04 14:43

cmsbuild · 2018-12-04T15:52:33Z

+1
Tested at: 0f86b4e
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25411/31980/summary.html

cmsbuild · 2018-12-04T15:52:37Z

Comparison job queued.

kpedro88 · 2018-12-04T16:44:53Z

RecoLocalCalo/HGCalRecAlgos/interface/HGCalImagingAlgo.h

-                 VerbosityLevel the_verbosity = pERROR) : vecDeltas_(vecDeltas_in), kappa_(kappa_in),
+                 VerbosityLevel the_verbosity = pERROR) :
+        W0threshold_(W0threshold_in),
+	positionDeltaRho_c_(positionDeltaRho_c_in),


inconsistent indentation

ok. I also fixed the indentation for other few lines in the same constructor

kpedro88 · 2018-12-04T16:47:06Z

RecoLocalCalo/HGCalRecAlgos/src/HGCalImagingAlgo.cc

+      float rhEnergy = v[idx].data.weight;
+      if(total_weight == 0. || rhEnergy == 0.) continue;
+      float W0_ = W0threshold_[thick];
+      Wi = (W0_ + log(rhEnergy/total_weight)) > 0 ? (W0_ + log(rhEnergy/total_weight)) : 0.;


Wi = std::max(W0_ + log(rhEnergy/total_weight), 0.);

kpedro88 · 2018-12-04T16:50:26Z

RecoLocalCalo/HGCalRecAlgos/src/HGCalImagingAlgo.cc

+      total_weight_log += Wi;
+    }
+    total_weight = total_weight_log;
+    x = x_log;


for silicon, why bother computing x and y values in the earlier loop that fills innerIndices if the values will get overwritten here?

perrotta · 2018-12-06T13:54:42Z

please test

cmsbuild · 2018-12-06T13:54:58Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32034/console Started: 2018/12/06 14:55

cmsbuild · 2018-12-06T16:28:18Z

+1
Tested at: 8359b22
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25411/32034/summary.html

cmsbuild · 2018-12-06T16:28:22Z

Comparison job queued.

cmsbuild · 2018-12-06T18:05:41Z

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

Comparison Summary:

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

perrotta · 2018-12-11T11:49:00Z

+1

Code changes are in line with the PR description
Changes in phase2 output are in the HGCal layer clusters, and propagate down to the higher level quantities

perrotta · 2018-12-11T11:54:09Z

@amartelli : since you did not want to integrate it here, I think that an adjustment that should be applied to all HGCal code in CMSSW (as soon as this PR gets merged) is to retrieve lastLayerEE and lastLayerFH from the geometry via RecHitTools.

As far as I can see, up to now only in RecoParticleFlow/PFClusterProducer/plugins/SimMappers/RealisticSimClusterMapper.cc those quantities are retrieved from RecHitTools, while everywhere else the same magic numbers are hardcoded instead. See for example:
https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_10_4_X_2018-12-10-2300&_filestring=&_string=lastLayerEE
and
https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_10_4_X_2018-12-10-2300&_filestring=&_string=lastLayerFH

amartelli · 2018-12-11T12:01:12Z

@perrotta. Yes agreed that replacing "magic numbers" with quantities retrieved from the geometry via RecHitTools has to be done everywhere and indeed there were already threads about this.
As you comment, this is a more general problem than specific to this PR.
I mention @rovere for the DPG, we'll discuss about a restyle of this for all HGCal code in CMSSW.
thanks

kpedro88 · 2018-12-11T15:13:28Z

+upgrade

cmsbuild · 2018-12-11T15:13:46Z

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 · 2018-12-11T15:18:32Z

I opened #25472 to track the hardcoded layer number issue

fabiocos · 2018-12-20T19:24:59Z

+1

arabella added 2 commits December 3, 2018 19:09

improvement in 2Dcl position for Si

6dd7ad3

remove unused variables

0f86b4e

cmsbuild added this to the CMSSW_10_4_X milestone Dec 4, 2018

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending upgrade-pending labels Dec 4, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 4, 2018

cmsbuild added tests-started and removed tests-pending labels Dec 4, 2018

cmsbuild added tests-approved and removed tests-started labels Dec 4, 2018

kpedro88 reviewed Dec 4, 2018

View reviewed changes

arabella added 2 commits December 4, 2018 18:16

fix indententation

4c117af

restyle code from PR review

a79098d

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-approved labels Dec 4, 2018

cmsbuild added tests-started and removed tests-pending labels Dec 6, 2018

cmsbuild added tests-approved and removed tests-started labels Dec 6, 2018

cmsbuild added comparison-available and removed comparison-pending labels Dec 6, 2018

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Dec 11, 2018

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Dec 11, 2018

kpedro88 mentioned this pull request Dec 11, 2018

Remove hardcoded lastLayerEE, lastLayerFH in HGCal algorithms #25472

Closed

smuzaffar modified the milestones: CMSSW_10_4_X, CMSSW_10_5_X Dec 20, 2018

cmsbuild added orp-approved and removed orp-pending labels Dec 20, 2018

cmsbuild merged commit bd57903 into cms-sw:master Dec 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improved position measurement for 2D clusters in HGCAL silicon sensors #25411

improved position measurement for 2D clusters in HGCAL silicon sensors #25411

amartelli commented Dec 4, 2018

cmsbuild commented Dec 4, 2018

cmsbuild commented Dec 4, 2018

cmsbuild commented Dec 4, 2018

perrotta commented Dec 4, 2018

cmsbuild commented Dec 4, 2018 •

edited

cmsbuild commented Dec 4, 2018

cmsbuild commented Dec 4, 2018

kpedro88 Dec 4, 2018

amartelli Dec 4, 2018

kpedro88 Dec 4, 2018

kpedro88 Dec 4, 2018

perrotta commented Dec 6, 2018

cmsbuild commented Dec 6, 2018 •

edited

cmsbuild commented Dec 6, 2018

cmsbuild commented Dec 6, 2018

cmsbuild commented Dec 6, 2018

perrotta commented Dec 11, 2018

perrotta commented Dec 11, 2018

amartelli commented Dec 11, 2018

kpedro88 commented Dec 11, 2018

cmsbuild commented Dec 11, 2018

kpedro88 commented Dec 11, 2018

fabiocos commented Dec 20, 2018

improved position measurement for 2D clusters in HGCAL silicon sensors #25411

improved position measurement for 2D clusters in HGCAL silicon sensors #25411

Conversation

amartelli commented Dec 4, 2018

cmsbuild commented Dec 4, 2018

cmsbuild commented Dec 4, 2018

cmsbuild commented Dec 4, 2018

perrotta commented Dec 4, 2018

cmsbuild commented Dec 4, 2018 • edited

cmsbuild commented Dec 4, 2018

cmsbuild commented Dec 4, 2018

kpedro88 Dec 4, 2018

Choose a reason for hiding this comment

amartelli Dec 4, 2018

Choose a reason for hiding this comment

kpedro88 Dec 4, 2018

Choose a reason for hiding this comment

kpedro88 Dec 4, 2018

Choose a reason for hiding this comment

perrotta commented Dec 6, 2018

cmsbuild commented Dec 6, 2018 • edited

cmsbuild commented Dec 6, 2018

cmsbuild commented Dec 6, 2018

cmsbuild commented Dec 6, 2018

perrotta commented Dec 11, 2018

perrotta commented Dec 11, 2018

amartelli commented Dec 11, 2018

kpedro88 commented Dec 11, 2018

cmsbuild commented Dec 11, 2018

kpedro88 commented Dec 11, 2018

fabiocos commented Dec 20, 2018

cmsbuild commented Dec 4, 2018 •

edited

cmsbuild commented Dec 6, 2018 •

edited