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] Corrections in noise vector in CLUE #32021

Merged
merged 7 commits into from
Nov 10, 2020
Merged

[HGCAL] Corrections in noise vector in CLUE #32021

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ad4acda
access noise same as the other vectors
apsallid Nov 2, 2020
49c1fc0
a comment to remember the extension
apsallid Nov 3, 2020
9555756
code checks
apsallid Nov 4, 2020
feb70d5
correcting HFNose out of range
apsallid Nov 4, 2020
7d64f80
droping type specifications
apsallid Nov 5, 2020
35d775c
use a cleaner syntax for the python config file
apsallid Nov 5, 2020
ed0b9b6
assign value of the object avoiding bindings
apsallid Nov 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions RecoLocalCalo/HGCalRecProducers/plugins/HGCalCLUEAlgo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,10 @@ class HGCalCLUEAlgoT : public HGCalClusteringAlgoBase {
maxNumberOfThickIndices_(ps.getParameter<unsigned>("maxNumberOfThickIndices")),
fcPerMip_(ps.getParameter<std::vector<double>>("fcPerMip")),
fcPerEle_(ps.getParameter<double>("fcPerEle")),
nonAgedNoises_(ps.getParameter<edm::ParameterSet>("noises").getParameter<std::vector<double>>("values")),
nonAgedNoises_(ps.getParameter<std::vector<double>>("noises")),
noiseMip_(ps.getParameter<edm::ParameterSet>("noiseMip").getParameter<double>("noise_MIP")),
use2x2_(ps.getParameter<bool>("use2x2")),
initialized_(false) {
// repeat same noises for CE-H as well
if (!isNose_) {
nonAgedNoises_.insert(std::end(nonAgedNoises_), std::begin(nonAgedNoises_), std::end(nonAgedNoises_));
}
}
initialized_(false) {}

~HGCalCLUEAlgoT() override {}

Expand Down Expand Up @@ -107,9 +102,7 @@ class HGCalCLUEAlgoT : public HGCalClusteringAlgoBase {
iDesc.add<unsigned>("maxNumberOfThickIndices", 6);
iDesc.add<std::vector<double>>("fcPerMip", {});
iDesc.add<double>("fcPerEle", 0.0);
edm::ParameterSetDescription descNestedNoises;
descNestedNoises.add<std::vector<double>>("values", {});
iDesc.add<edm::ParameterSetDescription>("noises", descNestedNoises);
iDesc.add<std::vector<double>>("noises", {});
edm::ParameterSetDescription descNestedNoiseMIP;
descNestedNoiseMIP.add<bool>("scaleByDose", false);
descNestedNoiseMIP.add<unsigned int>("scaleByDoseAlgo", 0);
Expand Down
34 changes: 20 additions & 14 deletions RecoLocalCalo/HGCalRecProducers/python/hgcalLayerClusters_cff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,36 @@

from RecoLocalCalo.HGCalRecProducers.HGCalUncalibRecHit_cfi import HGCalUncalibRecHit

from SimCalorimetry.HGCalSimProducers.hgcalDigitizer_cfi import fC_per_ele, hgceeDigitizer, hgchebackDigitizer, hfnoseDigitizer
from SimCalorimetry.HGCalSimProducers.hgcalDigitizer_cfi import fC_per_ele, HGCAL_noises, hgceeDigitizer, hgchebackDigitizer, hfnoseDigitizer

hgcalLayerClusters = hgcalLayerClusters_.clone()

hgcalLayerClusters.timeOffset = hgceeDigitizer.tofDelay
hgcalLayerClusters.plugin.dEdXweights = cms.vdouble(dEdX.weights)
hgcalLayerClusters.plugin.fcPerMip = cms.vdouble(HGCalUncalibRecHit.HGCEEConfig.fCPerMIP + HGCalUncalibRecHit.HGCHEFConfig.fCPerMIP)
hgcalLayerClusters.plugin.thicknessCorrection = cms.vdouble(HGCalRecHit.thicknessCorrection)
hgcalLayerClusters.plugin.sciThicknessCorrection = HGCalRecHit.sciThicknessCorrection
hgcalLayerClusters.plugin.deltasi_index_regemfac = HGCalRecHit.deltasi_index_regemfac
hgcalLayerClusters.plugin.fcPerEle = cms.double(fC_per_ele)
hgcalLayerClusters.plugin.noises = cms.PSet(refToPSet_ = cms.string('HGCAL_noises'))
hgcalLayerClusters.plugin.noiseMip = hgchebackDigitizer.digiCfg.noise
hgcalLayerClusters = hgcalLayerClusters_.clone(
timeOffset = hgceeDigitizer.tofDelay,
plugin = dict(
dEdXweights = dEdX.weights,
Copy link
Contributor

Choose a reason for hiding this comment

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

I tend to forget that assignment statements in Python do not copy objects (as in C++) but they create bindings between a target and an object. Therefore, if we assign (e.g.)

        dEdXweights = dEdX.weights,

and then modiffy dEdXweights afterwards, we are going to modify also the source dEdX.weight and all other configs which use it.

To avoid it one can make a deep copy, or assign the value of the object copied from (which was by the way what coded in the original suggestion of @slava77).

In this case one could therefore assign

        dEdXweights = dEdX.weights.value(),

and the same should also get applied to lines 17, 18, 19, 20, 23, 24 below, as well as in the corresponding lines within hgcalLayerClustersHFNose.

(As of now, none of those parameters get modified in our configs, and therefore there will be no practical effect on the actual configs. Let consider it as a safe practice to avoid possible future issues, in this case)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks a lot for the detailed explanation!

#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 + HGCalUncalibRecHit.HGCHEFConfig.fCPerMIP,
thicknessCorrection = HGCalRecHit.thicknessCorrection,
sciThicknessCorrection = HGCalRecHit.sciThicknessCorrection,
deltasi_index_regemfac = HGCalRecHit.deltasi_index_regemfac,
fcPerEle = fC_per_ele,
#Extending noises as fcPerMip, see comment above.
noises = HGCAL_noises.values + HGCAL_noises.values,
noiseMip = hgchebackDigitizer.digiCfg.noise
)
)

hgcalLayerClustersHFNose = hgcalLayerClusters_.clone(
detector = 'HFNose',
timeOffset = hfnoseDigitizer.tofDelay,
nHitsTime = cms.uint32(3),
nHitsTime = 3,
plugin = dict(
dEdXweights = dEdX.weightsNose,
maxNumberOfThickIndices = 3,
fcPerMip = HGCalUncalibRecHit.HGCHFNoseConfig.fCPerMIP,
thicknessCorrection = HGCalRecHit.thicknessNoseCorrection,
fcPerEle = fC_per_ele,
noises = dict(refToPSet_ = cms.string('HGCAL_noises')),
noises = HGCAL_noises.values,
noiseMip = hgchebackDigitizer.digiCfg.noise
)
)