Add HcalHPD type #17326

kpedro88 · 2017-01-30T16:56:55Z

Based on discussions in #17311, this PR adds an "HcalHPD" type to the HcalSiPMType enum. This is done in order to access per-channel photoelectronsToAnalog values for HPDs from the database, repurposing the SiPMParameters object. The hardcode conditions are updated appropriately. All other values for SiPM-related quantities will be set to dummy values, since they will not be used for HPDs.

I tested the effect of this PR locally by changing the photoelectronsToAnalog value in Hcal_Conditions_forGlobalTag_cff and observing changes in digi, rechit, etc. distributions. No changes in central workflows are expected.

Updates to the database conditions for HCAL will follow separately. Removal of the photoelectronsToAnalog Python parameter in hcalSimParameters_cfi may also follow separately.

attn: @abdoulline, @mariadalfonso

cmsbuild · 2017-01-30T16:57:13Z

A new Pull Request was created by @kpedro88 (Kevin Pedro) for CMSSW_9_0_X.

It involves the following packages:

CalibCalorimetry/HcalAlgos
CalibCalorimetry/HcalPlugins
CalibFormats/HcalObjects

@ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @mariadalfonso, @tocheng this is something you requested to watch as well.
@davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

kpedro88 · 2017-01-30T16:59:20Z

@cmsbuild please test

cmsbuild · 2017-01-30T16:59:36Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17507/console Started: 2017/01/30 19:28

cmsbuild · 2017-01-30T20:36:00Z

+1
Tested at: d86473b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17326/17507/summary.html

cmsbuild · 2017-01-30T20:36:03Z

Comparison job queued.

cmsbuild · 2017-01-30T22:25:00Z

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

abdoulline · 2017-02-07T06:35:55Z

@franzoni , @arunhep,

Giovanni and Arun, this is a technical amendment/extension to correctly describe what has already been submitted to Plan 0 conditions, namely
enum HcalSiPMType {..., HcalHPD=7}

https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/2808.html

Please, approve.

kpedro88 · 2017-02-09T04:19:13Z

@franzoni , @arunhep please sign

kpedro88 · 2017-02-10T13:49:02Z

@franzoni, @arunhep ping
@davidlange6 this is important for HCAL reco development and also will conflict with changes I have to make for Plan1, so please merge ASAP

ghellwig · 2017-02-10T14:56:49Z

CalibCalorimetry/HcalAlgos/src/HcalDbHardcode.cc

+    else theType = HcalHPD;
+  } else if (fId.genericSubdet() == HcalGenericDetId::HcalGenOuter) {
+    if(useHOUpgrade_) theType = HcalHOHamamatsu;
+    else theType = HcalHPD;
  }


Hi @kpedro88
sorry for coming to this only now.
I try to understand the change of the logic here:
It looks to me as if the new enum type is used in case of useHOUpgrade == false or useHEUpgrade == false, i.e. it is only valid for past runs?
Where is this type then used and what purpose does it serve?

We still have HPDs installed in the HB and HE. The hardcode conditions are intended to be used for development/testing for any HCAL scenario, not just future ones.

This type will be used to store HPD photoelectronToAnalog value in the database, as discussed in #17311 and https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/2808.html.

ghellwig · 2017-02-10T14:58:15Z

CalibCalorimetry/HcalPlugins/python/Hcal_Conditions_forGlobalTag_cff.py

@@ -35,7 +35,7 @@
        qieSlope      = cms.vdouble(0.912,0.917,0.922,0.923),
        mcShape       = cms.int32(125),
        recoShape     = cms.int32(105),
-        photoelectronsToAnalog = cms.double(0.0),
+        photoelectronsToAnalog = cms.double(0.3305),
        darkCurrent   = cms.vdouble(0.0),


The rest of the code changes looks like really just related to the additional enum, but what is the rationale behind this configuration parameter change?
Why does this value needs to be changed?

Previously this value was only stored in Python (https://github.com/cms-sw/cmssw/blob/CMSSW_9_0_X/SimCalorimetry/HcalSimProducers/python/hcalSimParameters_cfi.py). Now it will be used in Method 2 reconstruction as well as digi simulation, so it will be taken from the database as noted in my above comment and the PR description. Once this PR and the conditions updates are accepted, the Python config can be cleaned up to reduce duplication.

ghellwig · 2017-02-10T14:58:26Z

CalibCalorimetry/HcalPlugins/python/Hcal_Conditions_forGlobalTag_cff.py

@@ -48,7 +48,7 @@
        qieSlope      = cms.vdouble(0.912,0.916,0.920,0.922),
        mcShape       = cms.int32(125),
        recoShape     = cms.int32(105),
-        photoelectronsToAnalog = cms.double(0.0),
+        photoelectronsToAnalog = cms.double(0.3305),
        darkCurrent   = cms.vdouble(0.0),


ghellwig · 2017-02-10T15:57:59Z

@kpedro88 thanks for the clarifications!
I just have one final question for my understanding: As it seems, the class HcalDbHardcode is meant to avoid hard-coded values, but why is it still called hardcode? Is this just a historical choice or am I missing something here?
Sorry to ask such basic questions, but I am not yet very familiar with the HCAL code.

kpedro88 · 2017-02-10T16:00:39Z

Originally, all the values in HcalDbHardcode were actually hardcoded in C++. I've modernized it to be configurable in Python for many values (not all) over the past few years. The old name was retained because no one cared enough to change it.

ghellwig · 2017-02-10T16:02:50Z

+1
thanks @kpedro88

cmsbuild · 2017-02-10T16:03:09Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

slava77 · 2017-02-10T16:28:16Z

@davidlange6
this is one of the PRs needed for the HCAL M2 update and will need a sequence of events to happen from this PR to GT update to take effect on physics result.
Please merge or comment at your earliest.

davidlange6 · 2017-02-10T16:31:04Z

it was approved 28 minutes ago… yes, I’m aware of it

…

On Feb 10, 2017, at 5:28 PM, Slava Krutelyov ***@***.***> wrote: @davidlange6 this is one of the PRs needed for the HCAL M2 update and will need a sequence of events to happen from this PR to GT update to take effect on physics result. Please merge or comment at your earliest. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

davidlange6 · 2017-02-10T16:37:43Z

+1

kpedro88 added 2 commits January 30, 2017 07:05

add HPD to SiPM type list

c1f39e0

update hardcode conditions for HcalHPD type

d86473b

cmsbuild added this to the Next CMSSW_9_0_X milestone Jan 30, 2017

cmsbuild added alca-pending comparison-pending orp-pending pending-signatures tests-pending labels Jan 30, 2017

cmsbuild added tests-started and removed tests-pending labels Jan 30, 2017

cmsbuild added tests-approved and removed tests-started labels Jan 30, 2017

cmsbuild added comparison-available and removed comparison-pending labels Jan 30, 2017

ghellwig reviewed Feb 10, 2017

View reviewed changes

cmsbuild added alca-approved and removed alca-pending pending-signatures labels Feb 10, 2017

cmsbuild added the fully-signed label Feb 10, 2017

cmsbuild added orp-approved and removed orp-pending labels Feb 10, 2017

cmsbuild merged commit b5bc9b2 into cms-sw:CMSSW_9_0_X Feb 10, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add HcalHPD type #17326

Add HcalHPD type #17326

kpedro88 commented Jan 30, 2017

cmsbuild commented Jan 30, 2017

kpedro88 commented Jan 30, 2017

cmsbuild commented Jan 30, 2017 •

edited

Loading

cmsbuild commented Jan 30, 2017

cmsbuild commented Jan 30, 2017

cmsbuild commented Jan 30, 2017

abdoulline commented Feb 7, 2017

kpedro88 commented Feb 9, 2017

kpedro88 commented Feb 10, 2017

ghellwig Feb 10, 2017

kpedro88 Feb 10, 2017

ghellwig Feb 10, 2017

kpedro88 Feb 10, 2017

ghellwig Feb 10, 2017

ghellwig commented Feb 10, 2017

kpedro88 commented Feb 10, 2017

ghellwig commented Feb 10, 2017

cmsbuild commented Feb 10, 2017

slava77 commented Feb 10, 2017

davidlange6 commented Feb 10, 2017 via email

davidlange6 commented Feb 10, 2017

Add HcalHPD type #17326

Add HcalHPD type #17326

Conversation

kpedro88 commented Jan 30, 2017

cmsbuild commented Jan 30, 2017

kpedro88 commented Jan 30, 2017

cmsbuild commented Jan 30, 2017 • edited Loading

cmsbuild commented Jan 30, 2017

cmsbuild commented Jan 30, 2017

cmsbuild commented Jan 30, 2017

abdoulline commented Feb 7, 2017

kpedro88 commented Feb 9, 2017

kpedro88 commented Feb 10, 2017

ghellwig Feb 10, 2017

Choose a reason for hiding this comment

kpedro88 Feb 10, 2017

Choose a reason for hiding this comment

ghellwig Feb 10, 2017

Choose a reason for hiding this comment

kpedro88 Feb 10, 2017

Choose a reason for hiding this comment

ghellwig Feb 10, 2017

Choose a reason for hiding this comment

ghellwig commented Feb 10, 2017

kpedro88 commented Feb 10, 2017

ghellwig commented Feb 10, 2017

cmsbuild commented Feb 10, 2017

slava77 commented Feb 10, 2017

davidlange6 commented Feb 10, 2017 via email

davidlange6 commented Feb 10, 2017

cmsbuild commented Jan 30, 2017 •

edited

Loading