Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HGCAL RecHits Calibration for V16 geometry scenario #36728

Merged
merged 14 commits into from Jan 31, 2022
Merged

HGCAL RecHits Calibration for V16 geometry scenario #36728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c8fc8bb
rec hit calib for V16
apsallid Oct 10, 2021
424f5dd
reduce precision
apsallid Oct 17, 2021
cb4ac8a
update to latest v16 version
apsallid Jan 6, 2022
1d4a3bb
add regional em factors for V16
apsallid Jan 14, 2022
85870a5
corrections to weights and fCperMIP
apsallid Jan 18, 2022
b1e60ce
no need for manually dealing with last weight
apsallid Jan 18, 2022
a2c4777
use new modifier
apsallid Jan 19, 2022
a75738c
removing not needed modifier
apsallid Jan 20, 2022
b654f38
Revert "removing not needed modifier"
apsallid Jan 20, 2022
eb55bd2
dEdx per era
apsallid Jan 28, 2022
1a7c1c6
setting default as before
apsallid Jan 28, 2022
bb1066b
correcting to default without extra parameter
apsallid Jan 28, 2022
2e499e2
using layerWeights instead of dEdX
apsallid Jan 28, 2022
9c8c05a
avoid usage of dEdX in CLUE
apsallid Jan 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
79 changes: 79 additions & 0 deletions RecoLocalCalo/HGCalRecProducers/python/HGCalRecHit_cfi.py
View file
Open in desktop
Expand Up @@ -2,6 +2,63 @@
from SimCalorimetry.HGCalSimProducers.hgcalDigitizer_cfi import *
from RecoLocalCalo.HGCalRecProducers.HGCalUncalibRecHit_cfi import *

from Configuration.Eras.Modifier_phase2_hgcalV16_cff import phase2_hgcalV16

# There is no layer zero, while no average is taken for the last layer
dummy_weight = 0.0
def calcWeights(weightsPerLayer): res = [sum(wei)/2. for wei in zip(weightsPerLayer[:], weightsPerLayer[1:] + [weightsPerLayer[-1]])]; res[0] = dummy_weight; return res;


weightsPerLayer_V16 = cms.vdouble(dummy_weight,
5.55, # MeV
12.86,
9.4,
12.86,
9.4,
12.86,
9.4,
12.86,
9.4,
12.86,
9.4,
12.86,
9.4,
12.86,
9.4,
12.86,
9.4,
12.86,
13.54,
12.86,
13.54,
12.86,
13.54,
12.86,
13.54,
12.86,
58.63,
60.7,
60.7,
60.7,
60.7,
60.7,
60.7,
60.7,
60.7,
60.7,
60.7,
83.08,
83.08,
83.43,
83.61,
83.61,
83.61,
83.61,
83.61,
83.61,
83.61)


dEdX = cms.PSet(
# for v10 geometry
weights = cms.vdouble(0.0, # there is no layer zero
Expand Down Expand Up @@ -67,6 +124,22 @@
92.283895)
)


# for v16 geometry
dEdX_v16 = cms.PSet(
weights = cms.vdouble(calcWeights(weightsPerLayer_V16)),

weightsNose = cms.vdouble(0.0, # there is no layer zero
39.500245, # MeV
39.756638,
39.756638,
39.756638,
39.756638,
66.020266,
92.283895,
92.283895)
)

# HGCAL rechit producer
HGCalRecHit = cms.EDProducer(
"HGCalRecHitProducer",
Expand Down Expand Up @@ -133,3 +206,9 @@
phase2_hgcalV10.toModify( HGCalRecHit , thicknessCorrection = [0.77, 0.77, 0.77, 0.84, 0.84, 0.84] , sciThicknessCorrection = 0.90 )

phase2_hfnose.toModify( HGCalRecHit , thicknessNoseCorrection = [0.58,0.58,0.58])

phase2_hgcalV16.toModify(HGCalRecHit,
thicknessCorrection = [0.75, 0.76, 0.75, 0.85, 0.85, 0.84] ,
sciThicknessCorrection = 0.69,
layerWeights = dEdX_v16.weights)

19 changes: 12 additions & 7 deletions RecoLocalCalo/HGCalRecProducers/python/HGCalUncalibRecHit_cfi.py
View file
Open in desktop
Expand Up @@ -2,7 +2,8 @@

from SimCalorimetry.HGCalSimProducers.hgcalDigitizer_cfi import hgceeDigitizer, hgchefrontDigitizer, hgchebackDigitizer, hfnoseDigitizer

fCPerMIP_v10 = cms.vdouble(2.06,3.43,5.15) #120um, 200um, 300um
fCPerMIP_mpv = cms.vdouble(1.25,2.57,3.88) #120um, 200um, 300um
fCPerMIP_mean = cms.vdouble(2.06,3.43,5.15) #120um, 200um, 300um

# HGCAL producer of rechits starting from digis
HGCalUncalibRecHit = cms.EDProducer(
Expand All @@ -26,7 +27,7 @@
tdcSaturation = hgceeDigitizer.digiCfg.feCfg.tdcSaturation_fC,
tdcOnset = hgceeDigitizer.digiCfg.feCfg.tdcOnset_fC,
toaLSB_ns = hgceeDigitizer.digiCfg.feCfg.toaLSB_ns,
fCPerMIP = cms.vdouble(1.25,2.57,3.88) #100um, 200um, 300um
fCPerMIP = fCPerMIP_mpv
),

HGCHEFConfig = cms.PSet(
Expand All @@ -39,7 +40,7 @@
tdcSaturation = hgchefrontDigitizer.digiCfg.feCfg.tdcSaturation_fC,
tdcOnset = hgchefrontDigitizer.digiCfg.feCfg.tdcOnset_fC,
toaLSB_ns = hgchefrontDigitizer.digiCfg.feCfg.toaLSB_ns,
fCPerMIP = cms.vdouble(1.25,2.57,3.88) #100um, 200um, 300um
fCPerMIP = fCPerMIP_mpv
),

HGCHEBConfig = cms.PSet(
Expand All @@ -65,18 +66,22 @@
tdcSaturation = hfnoseDigitizer.digiCfg.feCfg.tdcSaturation_fC,
tdcOnset = hfnoseDigitizer.digiCfg.feCfg.tdcOnset_fC,
toaLSB_ns = hfnoseDigitizer.digiCfg.feCfg.toaLSB_ns,
fCPerMIP = cms.vdouble(1.25,2.57,3.88) #100um, 200um, 300um
fCPerMIP = fCPerMIP_mpv
),

algo = cms.string("HGCalUncalibRecHitWorkerWeights")
)

from Configuration.Eras.Modifier_phase2_hgcalV10_cff import phase2_hgcalV10
phase2_hgcalV10.toModify( HGCalUncalibRecHit.HGCEEConfig , fCPerMIP = fCPerMIP_v10 )
phase2_hgcalV10.toModify( HGCalUncalibRecHit.HGCHEFConfig , fCPerMIP = fCPerMIP_v10 )
phase2_hgcalV10.toModify( HGCalUncalibRecHit.HGCEEConfig , fCPerMIP = fCPerMIP_mean )
phase2_hgcalV10.toModify( HGCalUncalibRecHit.HGCHEFConfig , fCPerMIP = fCPerMIP_mean )

from Configuration.Eras.Modifier_phase2_hgcalV16_cff import phase2_hgcalV16
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add a new Configuration.Eras.Modifier_phase2_hgcalV16_cff, at the moment is missing and the test is failing

File "/cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36728/22058/CMSSW_12_3_X_2022-01-27-2300/python/RecoLocalCalo/HGCalRecProducers/HGCalUncalibRecHit_cfi.py", line 79, in <module>
    from Configuration.Eras.Modifier_phase2_hgcalV16_cff import phase2_hgcalV16
ModuleNotFoundError: No module named 'Configuration.Eras.Modifier_phase2_hgcalV16_cff'

phase2_hgcalV16.toModify( HGCalUncalibRecHit.HGCEEConfig , fCPerMIP = fCPerMIP_mean )
phase2_hgcalV16.toModify( HGCalUncalibRecHit.HGCHEFConfig , fCPerMIP = fCPerMIP_mean )
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these two configs the same as phase2_hgcalV10? If they are the same, I believe we don't need them because Era of Phase2C17I13M9 is based on C12 ==> C11 (but excludes HGCal V12) ==> C9 (still includes phase2_hgcalV10 and phase2_hgcalV11)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, they are the same. So, you say that Phase2C17I13M9 includes phase2_hgcalV10 and will pick up the relevant values of the phase2_hgcalV10 if phase2_hgcalV16 is not present? Ok, then yes I will remove it now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ciao @srimanob this is a part that I am aware of and that I do not particularly like about eras and their chaining.
I did not suggest that change so that the choice would be explicit and to ease the process of removing old eras from the release in the future.
If we now want to obsolete phase2_hgcalV10 we will need additional code to handle that.
I understand the usefulness of chaining and combining eras together, but I do not see it working in a situation like the one for HGCAL, in which we need to support, in the end, one detector and one configuration.
I'd therefore be in favour of reverting the last commit from Andreas.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @rovere
I have no issue if you want to keep. I see that phase2_hgcalV10 is used in several places so I understand that it is the basic customization you will have. So changing when clean-up is OK. But if you would like to keep it for this part first, I am also fine too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ciao @srimanob yes, V10 is used as a baseline pretty much everywhere. After some years of development, I'd have not coupled eras at all. In the end, reverting the change back will add just a couple of CPU cycles redoing what V10 already did under the hood.
I'm still in favour of having the explicit call to V16.

The cleaning will not be trivial, I realize. As soon as the situation from the geometry point of view is more stable, we will address that, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK great, let me revert the last commit and push it.


from Configuration.Eras.Modifier_phase2_hfnose_cff import phase2_hfnose
phase2_hfnose.toModify( HGCalUncalibRecHit.HGCHFNoseConfig ,
isSiFE = True ,
fCPerMIP = fCPerMIP_v10
fCPerMIP = fCPerMIP_mean
)
6 changes: 3 additions & 3 deletions RecoLocalCalo/HGCalRecProducers/python/hgcalLayerClusters_cff.py
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@

from RecoLocalCalo.HGCalRecProducers.hgcalLayerClusters_cfi import hgcalLayerClusters as hgcalLayerClusters_

from RecoLocalCalo.HGCalRecProducers.HGCalRecHit_cfi import dEdX, HGCalRecHit
from RecoLocalCalo.HGCalRecProducers.HGCalRecHit_cfi import HGCalRecHit

from RecoLocalCalo.HGCalRecProducers.HGCalUncalibRecHit_cfi import HGCalUncalibRecHit

Expand All @@ -11,7 +11,7 @@
hgcalLayerClusters = hgcalLayerClusters_.clone(
timeOffset = hgceeDigitizer.tofDelay,
plugin = dict(
dEdXweights = dEdX.weights.value(),
dEdXweights = HGCalRecHit.layerWeights.value(),
#With the introduction of 7 regional factors (6 for silicon plus 1 for scintillator),
#we extend fcPerMip (along with noises below) so that it is guaranteed that they have 6 entries.
fcPerMip = HGCalUncalibRecHit.HGCEEConfig.fCPerMIP.value() + HGCalUncalibRecHit.HGCHEFConfig.fCPerMIP.value(),
Expand All @@ -30,7 +30,7 @@
timeOffset = hfnoseDigitizer.tofDelay.value(),
nHitsTime = 3,
plugin = dict(
dEdXweights = dEdX.weightsNose.value(),
dEdXweights = HGCalRecHit.layerNoseWeights.value(),
maxNumberOfThickIndices = 3,
fcPerMip = HGCalUncalibRecHit.HGCHFNoseConfig.fCPerMIP.value(),
thicknessCorrection = HGCalRecHit.thicknessNoseCorrection.value(),
Expand Down