Run2-hcx172 Modify 2 scripts for AlCaReco #22835
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@cmsbuild Please test |
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22835/4217 |
|
The tests are being triggered in jenkins. |
|
A new Pull Request was created by @bsunanda for master. It involves the following packages: Calibration/HcalAlCaRecoProducers @ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @lpernie can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
@bsunanda Sunanda, I think the following two configs might also require similar update(s) : |
|
...And we need these for the beginning of the data taking with 10_1_1... |
|
Comparison is ready Comparison Summary:
|
perrotta
reviewed
Apr 5, 2018
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| # Configuration parameters for Method 0 | |||
| m0Parameters = cms.PSet( | |||
| firstSample = cms.int32(4), | |||
There was a problem hiding this comment.
@bsunanda : do I understand correctly that this parameter (firstSample) is removed from the config because it is actually not needed by the HBHEPhase1Reconstructor?
There was a problem hiding this comment.
Yes you are absolutely correct. The reconstructor (for Method0) uses the new variable firstSampleShift rather than firstSample. It was by mistake kept in HBHEMethod0Parameters_cfi and the new parameter was added earlier in HBHEReconstructor_cfi.py. This is now corrected for so that calibration cff's can be correctly written
deguio
reviewed
Apr 5, 2018
| @@ -39,8 +39,7 @@ | |||
| setLegacyFlagsQIE11 = cms.bool(False), | |||
| ) | |||
|
|
|||
| hbherecoNoise.algorithm.firstSample = 0 | |||
| hbherecoNoise.algorithm.samplesToAdd = 4 | |||
| hbherecoNoise.algorithm.firstSampleShift = -100 # to set reco window at very beginning of the TS array | |||
There was a problem hiding this comment.
- is this supposed to work even with
recoParamsFromDB = True? - where the end of the integration window is defined?
|
Federico,
(1)
as Igor has mentioned elsewhere, the logic is here:
(a) tsFromDB determines how SOI is evaluated
http://cmslxr.fnal.gov/source/RecoLocalCalo/HcalRecProducers/src/HBHEPhase1Reconstructor.cc#0499
(b) recoParams used below in the algo (if in HBHEPhase1Reconstructor
recoParamsFromDB = True)
http://cmslxr.fnal.gov/source/RecoLocalCalo/HcalRecAlgos/src/SimpleHBHEPhase1Algo.cc#0071
but it doesn't affect "ibeg" = 0. So the answer is yes.
In principle, we have a (limited) goal to make ALCARECOHcal work as we
want with a certain/specific config parameters and it's what we have in
this PR.
(2)
As far as I understand, the end of reco window comes from DB
(for recoParamsFromDB = True)
http://cmslxr.fnal.gov/source/RecoLocalCalo/HcalRecAlgos/src/SimpleHBHEPhase1Algo.cc#0074
NB: we have to ask Igor to put together a table of these
parameters/switches (tsFromDB/recoParamsFromDB/fistSampleShift/...)
interaction effect on the final M0 parameters, similar to what he did for
HF SOI here
http://cmslxr.fnal.gov/source/RecoLocalCalo/HcalRecProducers/python/hfprereco_cfi.py
…On Thu, 5 Apr 2018, Federico De Guio wrote:
@deguio commented on this pull request.
__________________________________________________________________________________________________
In Calibration/HcalAlCaRecoProducers/python/ALCARECOHcalCalMinBiasNoise_cff.py:
> @@ -39,8 +39,7 @@
setLegacyFlagsQIE11 = cms.bool(False),
)
-hbherecoNoise.algorithm.firstSample = 0
-hbherecoNoise.algorithm.samplesToAdd = 4
+hbherecoNoise.algorithm.firstSampleShift = -100 # to set reco window at very beginning of the TS
array
* is this supposed to work even with recoParamsFromDB = True?
* where the end of the integration window is defined?
@igv4321 @abdoulline
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the
thread.[AEx02k9mGyt470y_taUM2dvFHwQLUAHEks5tlmPhgaJpZM4TGRPg.gif]
|
|
Hi Salavat,
If you like, you can put the following comments into
"HBHEMethod0Parameters_cfi.py". I am not completely
sure though about "correctionPhaseNS" meaning. That
is, if you increase this value, will the code assume that
the HPD signal is delayed or advanced?
Regards,
Igor
#
# Behavior of "Method 0" depends on the following top-level
# HBHEPhase1Reconstructor parameters:
#
# (bool) tsFromDB
# (bool) recoParamsFromDB
#
# and on the following parameters inside the "algorithm" parameter set:
#
# (int) firstSampleShift
# (int) samplesToAdd
# (bool) correctForPhaseContainment
# (double) correctionPhaseNS
#
# The role of "tsFromDB" parameter is to specify whether the
# "sample of interest" (SOI) should be taken from the data frame
# (in case tsFromDB is False) or from the database (tsFromDB is True).
# Once SOI is determined in this manner, the time slice for starting
# the ADC charge summation (TSS) is determined by adding SOI and
# "firstSampleShift". "firstSampleShift" can be negative, and if
# TSS = SOI + firstSampleShift ends up negative, TSS is reset to 0.
# If you want to start summation from time slice 0, just set
# "firstSampleShift" to some negative number with large magnitude.
#
# In the old reco code, it used to be possible to configure the TSS
# using a parameter value. This is no longer the case for Phase 1
# reconstructor.
#
# The values of the remaining M0 parameters, "samplesToAdd",
# "correctForPhaseContainment", and "correctionPhaseNS", will be
# taken from the configuration file only if "recoParamsFromDB"
# is set to False. If "recoParamsFromDB" is True, the config
# file values of these parameters are ignored, and these values
# are instead taken from the database. The "samplesToAdd" parameter
# defines how many contiguous time slices will be used to calculate
# the charge. TSS + samplesToAdd should not exceed the number of
# time slices in the data frame, otherwise M0 results will be
# unreliable.
#
# Parameter "correctForPhaseContainment" specifies whether
# a correction should be made for incomplete HPD signal collection
# inside the summed time slices, and "correctionPhaseNS" specifies
# the delay (in ns) of the HPD signal w.r.t. the ADC digitization
# clock edge.
#
…On 04/05/2018 03:54 PM, Salavat Abdoulline wrote:
Federico,
(1)
as Igor has mentioned elsewhere, the logic is here:
(a) tsFromDB determines how SOI is evaluated
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecProducers%2Fsrc%2FHBHEPhase1Reconstructor.cc%230499&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=mU7beG9VEcRhSIcgi5lAFC4cTZdKmUsqHhsOV5AghME%3D&reserved=0
(b) recoParams used below in the algo (if in HBHEPhase1Reconstructor recoParamsFromDB = True)
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecAlgos%2Fsrc%2FSimpleHBHEPhase1Algo.cc%230071&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=e%2BQygJioal8xCi3OVrbFcdSdVddU0wuSram692rAzdM%3D&reserved=0
but it doesn't affect "ibeg" = 0. So the answer is yes.
In principle, we have a (limited) goal to make ALCARECOHcal work as we want with a certain/specific config parameters and it's what we have in this PR.
(2)
As far as I understand, the end of reco window comes from DB
(for recoParamsFromDB = True)
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecAlgos%2Fsrc%2FSimpleHBHEPhase1Algo.cc%230074&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=Ou7iBUHmgmM38ScuFOSUVbDEcQHEhUUzmgTN0pDFnko%3D&reserved=0
NB: we have to ask Igor to put together a table of these parameters/switches (tsFromDB/recoParamsFromDB/fistSampleShift/...) interaction effect on the final M0 parameters, similar to what he did for
HF SOI here
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecProducers%2Fpython%2Fhfprereco_cfi.py&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=YDg1Ma2Ff%2F6iWSNKNIow5NvJ1Ws%2BBg%2F3Tcf81jqZIU8%3D&reserved=0
On Thu, 5 Apr 2018, Federico De Guio wrote:
>
> @deguio commented on this pull request.
>
> __________________________________________________________________________________________________
>
> In Calibration/HcalAlCaRecoProducers/python/ALCARECOHcalCalMinBiasNoise_cff.py:
>
> > @@ -39,8 +39,7 @@
> setLegacyFlagsQIE11 = cms.bool(False),
> )
>
> -hbherecoNoise.algorithm.firstSample = 0
> -hbherecoNoise.algorithm.samplesToAdd = 4
> +hbherecoNoise.algorithm.firstSampleShift = -100 # to set reco window at very beginning of the TS array
> * is this supposed to work even with recoParamsFromDB = True?
> * where the end of the integration window is defined?
>
> @igv4321 @abdoulline
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the
> thread.[AEx02k9mGyt470y_taUM2dvFHwQLUAHEks5tlmPhgaJpZM4TGRPg.gif]
>
>
>
|
|
@bsunanda @abdoulline please specify if you plan to add into HBHEMethod0Parameters_cfi.py the comment lines listed in #22835 (comment) within this PR |
|
@lpernie I notice you signed the backport PR and not this one for the master, which is identical)ì: maybe you wanted to do the opposite (or sign both them)... |
|
OK thank you, Igor. It's worth putting this description at the next
(editing) occasion.
Cannot immediately recall correctionPhaseN sign/direction... Will take a
closer look.
…On Fri, 6 Apr 2018, Igor Volobouev wrote:
Hi Salavat,
If you like, you can put the following comments into
"HBHEMethod0Parameters_cfi.py". I am not completely
sure though about "correctionPhaseNS" meaning. That
is, if you increase this value, will the code assume that
the HPD signal is delayed or advanced?
Regards,
Igor
#
# Behavior of "Method 0" depends on the following top-level
# HBHEPhase1Reconstructor parameters:
#
# (bool) tsFromDB
# (bool) recoParamsFromDB
#
# and on the following parameters inside the "algorithm" parameter set:
#
# (int) firstSampleShift
# (int) samplesToAdd
# (bool) correctForPhaseContainment
# (double) correctionPhaseNS
#
# The role of "tsFromDB" parameter is to specify whether the
# "sample of interest" (SOI) should be taken from the data frame
# (in case tsFromDB is False) or from the database (tsFromDB is True).
# Once SOI is determined in this manner, the time slice for starting
# the ADC charge summation (TSS) is determined by adding SOI and
# "firstSampleShift". "firstSampleShift" can be negative, and if
# TSS = SOI + firstSampleShift ends up negative, TSS is reset to 0.
# If you want to start summation from time slice 0, just set
# "firstSampleShift" to some negative number with large magnitude.
#
# In the old reco code, it used to be possible to configure the TSS
# using a parameter value. This is no longer the case for Phase 1
# reconstructor.
#
# The values of the remaining M0 parameters, "samplesToAdd",
# "correctForPhaseContainment", and "correctionPhaseNS", will be
# taken from the configuration file only if "recoParamsFromDB"
# is set to False. If "recoParamsFromDB" is True, the config
# file values of these parameters are ignored, and these values
# are instead taken from the database. The "samplesToAdd" parameter
# defines how many contiguous time slices will be used to calculate
# the charge. TSS + samplesToAdd should not exceed the number of
# time slices in the data frame, otherwise M0 results will be
# unreliable.
#
# Parameter "correctForPhaseContainment" specifies whether
# a correction should be made for incomplete HPD signal collection
# inside the summed time slices, and "correctionPhaseNS" specifies
# the delay (in ns) of the HPD signal w.r.t. the ADC digitization
# clock edge.
#
On 04/05/2018 03:54 PM, Salavat Abdoulline wrote:
>
> Federico,
>
> (1)
> as Igor has mentioned elsewhere, the logic is here:
>
> (a) tsFromDB determines how SOI is evaluated
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecProducers%2Fsrc%2FHBHEPhase1Reconstructor.cc%230499&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=mU7beG9VEcRhSIcgi5lAFC4cTZdKmUsqHhsOV5AghME%3D&reserved=0
>
> (b) recoParams used below in the algo (if in HBHEPhase1Reconstructor
> recoParamsFromDB = True)
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecAlgos%2Fsrc%2FSimpleHBHEPhase1Algo.cc%230071&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=e%2BQygJioal8xCi3OVrbFcdSdVddU0wuSram692rAzdM%3D&reserved=0
>
> but it doesn't affect "ibeg" = 0. So the answer is yes.
> In principle, we have a (limited) goal to make ALCARECOHcal work as we
> want with a certain/specific config parameters and it's what we have in
> this PR.
>
> (2)
> As far as I understand, the end of reco window comes from DB
> (for recoParamsFromDB = True)
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecAlgos%2Fsrc%2FSimpleHBHEPhase1Algo.cc%230074&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=Ou7iBUHmgmM38ScuFOSUVbDEcQHEhUUzmgTN0pDFnko%3D&reserved=0
>
>
>
> NB: we have to ask Igor to put together a table of these
> parameters/switches (tsFromDB/recoParamsFromDB/fistSampleShift/...)
> interaction effect on the final M0 parameters, similar to what he did for
> HF SOI here
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecProducers%2Fpython%2Fhfprereco_cfi.py&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=YDg1Ma2Ff%2F6iWSNKNIow5NvJ1Ws%2BBg%2F3Tcf81jqZIU8%3D&reserved=0
>
>
>
>
>
> On Thu, 5 Apr 2018, Federico De Guio wrote:
>
> >
> > @deguio commented on this pull request.
> >
> > __________________________________________________________________________________________________
> >
> > In
> > Calibration/HcalAlCaRecoProducers/python/ALCARECOHcalCalMinBiasNoise_cff.py:
> >
> > > @@ -39,8 +39,7 @@
> > setLegacyFlagsQIE11 = cms.bool(False),
> > )
> >
> > -hbherecoNoise.algorithm.firstSample = 0
> > -hbherecoNoise.algorithm.samplesToAdd = 4
> > +hbherecoNoise.algorithm.firstSampleShift = -100 # to set reco window at
> > very beginning of the TS array
> > * is this supposed to work even with recoParamsFromDB = True?
> > * where the end of the integration window is defined?
> >
> > @igv4321 @abdoulline
> >
> > —
> > You are receiving this because you were mentioned.
> > Reply to this email directly, view it on GitHub, or mute the
> > thread.[AEx02k9mGyt470y_taUM2dvFHwQLUAHEks5tlmPhgaJpZM4TGRPg.gif]
> >
> >
> >
|
|
+1
|
|
thanks @abdoulline, @bsunanda, @igv4321
it doesn't make sense to apply containment corrections for measuring the PED. also: can somebody please make sure that the logic is compliant with the values we currently have in DB? this should be the content of the tag we are using: thanks in advance, |
|
NB: increasing correctionPhaseNS moves reco window to the left wrt signal
(or actually delaying the signal wrt reco window) in the
HcalPlusContainment calculation, so:
(1) cutting (more) signal's right shoulder (for finite reco window
of 2TS for instance)
(2) thus increasing the correction factor:
http://cmslxr.fnal.gov/source/CalibCalorimetry/HcalAlgos/src/HcalPulseContainmentAlgo.cc#0072
…On Fri, 6 Apr 2018, Salavat Abdoulline wrote:
OK thank you, Igor. It's worth putting this description at the next
(editing) occasion.
Cannot immediately recall correctionPhaseN sign/direction... Will take a
closer look.
On Fri, 6 Apr 2018, Igor Volobouev wrote:
> Hi Salavat,
>
> If you like, you can put the following comments into
> "HBHEMethod0Parameters_cfi.py". I am not completely
> sure though about "correctionPhaseNS" meaning. That
> is, if you increase this value, will the code assume that
> the HPD signal is delayed or advanced?
>
> Regards,
> Igor
>
> #
> # Behavior of "Method 0" depends on the following top-level
> # HBHEPhase1Reconstructor parameters:
> #
> # (bool) tsFromDB
> # (bool) recoParamsFromDB
> #
> # and on the following parameters inside the "algorithm" parameter set:
> #
> # (int) firstSampleShift
> # (int) samplesToAdd
> # (bool) correctForPhaseContainment
> # (double) correctionPhaseNS
> #
> # The role of "tsFromDB" parameter is to specify whether the
> # "sample of interest" (SOI) should be taken from the data frame
> # (in case tsFromDB is False) or from the database (tsFromDB is True).
> # Once SOI is determined in this manner, the time slice for starting
> # the ADC charge summation (TSS) is determined by adding SOI and
> # "firstSampleShift". "firstSampleShift" can be negative, and if
> # TSS = SOI + firstSampleShift ends up negative, TSS is reset to 0.
> # If you want to start summation from time slice 0, just set
> # "firstSampleShift" to some negative number with large magnitude.
> #
> # In the old reco code, it used to be possible to configure the TSS
> # using a parameter value. This is no longer the case for Phase 1
> # reconstructor.
> #
> # The values of the remaining M0 parameters, "samplesToAdd",
> # "correctForPhaseContainment", and "correctionPhaseNS", will be
> # taken from the configuration file only if "recoParamsFromDB"
> # is set to False. If "recoParamsFromDB" is True, the config
> # file values of these parameters are ignored, and these values
> # are instead taken from the database. The "samplesToAdd" parameter
> # defines how many contiguous time slices will be used to calculate
> # the charge. TSS + samplesToAdd should not exceed the number of
> # time slices in the data frame, otherwise M0 results will be
> # unreliable.
> #
> # Parameter "correctForPhaseContainment" specifies whether
> # a correction should be made for incomplete HPD signal collection
> # inside the summed time slices, and "correctionPhaseNS" specifies
> # the delay (in ns) of the HPD signal w.r.t. the ADC digitization
> # clock edge.
> #
>
>
> On 04/05/2018 03:54 PM, Salavat Abdoulline wrote:
> >
> > Federico,
> >
> > (1)
> > as Igor has mentioned elsewhere, the logic is here:
> >
> > (a) tsFromDB determines how SOI is evaluated
> > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecProducers%2Fsrc%2FHBHEPhase1Reconstructor.cc%230499&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=mU7beG9VEcRhSIcgi5lAFC4cTZdKmUsqHhsOV5AghME%3D&reserved=0
> >
> > (b) recoParams used below in the algo (if in HBHEPhase1Reconstructor
> > recoParamsFromDB = True)
> > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecAlgos%2Fsrc%2FSimpleHBHEPhase1Algo.cc%230071&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=e%2BQygJioal8xCi3OVrbFcdSdVddU0wuSram692rAzdM%3D&reserved=0
> >
> > but it doesn't affect "ibeg" = 0. So the answer is yes.
> > In principle, we have a (limited) goal to make ALCARECOHcal work as we
> > want with a certain/specific config parameters and it's what we have in
> > this PR.
> >
> > (2)
> > As far as I understand, the end of reco window comes from DB
> > (for recoParamsFromDB = True)
> > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecAlgos%2Fsrc%2FSimpleHBHEPhase1Algo.cc%230074&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=Ou7iBUHmgmM38ScuFOSUVbDEcQHEhUUzmgTN0pDFnko%3D&reserved=0
> >
> >
> >
> > NB: we have to ask Igor to put together a table of these
> > parameters/switches (tsFromDB/recoParamsFromDB/fistSampleShift/...)
> > interaction effect on the final M0 parameters, similar to what he did
> > for
> > HF SOI here
> > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcmslxr.fnal.gov%2Fsource%2FRecoLocalCalo%2FHcalRecProducers%2Fpython%2Fhfprereco_cfi.py&data=02%7C01%7Ci.volobouev%40ttu.edu%7Cb8b100eac1084dbf96fa08d59b37726b%7C178a51bf8b2049ffb65556245d5c173c%7C0%7C0%7C636585584808297792&sdata=YDg1Ma2Ff%2F6iWSNKNIow5NvJ1Ws%2BBg%2F3Tcf81jqZIU8%3D&reserved=0
> >
> >
> >
> >
> >
> > On Thu, 5 Apr 2018, Federico De Guio wrote:
> >
> > >
> > > @deguio commented on this pull request.
> > >
> > > __________________________________________________________________________________________________
> > >
> > > In
> > > Calibration/HcalAlCaRecoProducers/python/ALCARECOHcalCalMinBiasNoise_cff.py:
> > >
> > > > @@ -39,8 +39,7 @@
> > > setLegacyFlagsQIE11 = cms.bool(False),
> > > )
> > >
> > > -hbherecoNoise.algorithm.firstSample = 0
> > > -hbherecoNoise.algorithm.samplesToAdd = 4
> > > +hbherecoNoise.algorithm.firstSampleShift = -100 # to set reco window
> > > at very beginning of the TS array
> > > * is this supposed to work even with recoParamsFromDB = True?
> > > * where the end of the integration window is defined?
> > >
> > > @igv4321 @abdoulline
> > >
> > > —
> > > You are receiving this because you were mentioned.
> > > Reply to this email directly, view it on GitHub, or mute the
> > > thread.[AEx02k9mGyt470y_taUM2dvFHwQLUAHEks5tlmPhgaJpZM4TGRPg.gif]
> > >
> > >
> > >
>
>
>
|
|
Federico, we can probably discuss it more extensively elsewhere.
Just my two cents:
(1) reco window size in DB is reasonable for all eras.
It may not be advantageous to extend HB/HE reco window from default 2 to 3
(especially in HE)? Old/obsolete sampleToadd=4 came as a legacy from Run 1.
For low-energy stuff + 2TS window energy pulse corrections are
non-negligible and needs to be applied both to signal and noise (as signal
is close to noise in some regions in NZS dataset) to keep them at the same
ground. It's done by default (from DB) for M0.
For non-applying them explicitly - should be some good reason? Probably
Olga have them?
And this PR is minimal (not many explicit re-definitions of reco params),
which I consider as an advantage.
(2)
NB: Olga's criteria came from me (at some point over Easter as an educated guess), please don't
consider them as a rock-solid reference. They just suggested to somehow separate non-signal range from the signal one.
For ALCARECO the chosen M0 reco window(s) _may not_ necessarily be the same
as for regular RECO. But right now, in this PR they are identical (for
signal). I think it's OK.
…On Fri, 6 Apr 2018, Federico De Guio wrote:
thanks @abdoulline, @bsunanda, @igv4321
to me it looks like for the ALCARECOHcalCalMinBiasNoise_cff.py we want to set:
* recoParamsFromDB = false
* specify by hand the summation window (samplesToAdd = 3?)
* turn off the containment correction
* make sure that correctForPhaseContainment and correctionPhaseNS take sensible values
it doesn't make sense to apply containment corrections for measuring the PED.
reporting here the criteria indicated by Olga:
- HBHE
- Noise: TS012
- Signal: TS345
- HF
- Noise: TS0
- Signal: TS1
- HO
- Noise: TS012
- Signal: TS3456
also: can somebody please make sure that the logic is compliant with the values we currently have in DB? this should
be the content of the tag we are using:
https://twiki.cern.ch/twiki/pub/CMS/HcalRecoParamsTags2011/HcalRecoParams_2018_v1.0_data.txt
thanks in advance,
F.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the
thread.[AEx02u2jdqcqI-KQNGUDdVpaLDVAm1aSks5tlxYjgaJpZM4TGRPg.gif]
|
|
@deguio What would be incorrect if we apply default corrections (so without additional customizations for recoParamsFromDB = false) both to pedestals and signal? Phi-symmetry is evaluating their (signal vs "pedestal") dispersion difference, right? |
|
+1 |
cmsbuild
added
alca-approved
fully-signed
and removed
alca-pending
pending-signatures
labels
|
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) |
|
+1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Take care of NZS reconstruction sequences for 2018