include dark current contribution in M2 pedestal constraint for Phase2 #20422
Conversation
|
The code-checks are being triggered in jenkins. |
|
A new Pull Request was created by @kpedro88 (Kevin Pedro) for master. It involves the following packages: HLTrigger/Configuration @perrotta, @cmsbuild, @silviodonato, @slava77, @Martin-Grunewald, @fwyzard can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
kpedro88
force-pushed
the
M2IncludeDCPedCon930p5
branch
from
dfc08b4
to
1a4b6b4
Compare
|
The code-checks are being triggered in jenkins. |
|
Pull request #20422 was updated. @perrotta, @cmsbuild, @silviodonato, @slava77, @Martin-Grunewald, @fwyzard can you please check and sign again. |
|
please test |
|
The tests are being triggered in jenkins. |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20422/563 Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/PR-20422/563/git-diff.patch You can run |
cmsbuild
added
code-checks-pending
tests-pending
and removed
code-checks-rejected
tests-started
labels
|
The code-checks are being triggered in jenkins. |
|
Pull request #20422 was updated. @perrotta, @cmsbuild, @silviodonato, @slava77, @Martin-Grunewald, @fwyzard can you please check and sign again. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
Minor changes in HCAL reconstruction for Phase2 workflows, as expected; no changes otherwise. Examples:
|
|
+1 |
|
Further demonstration of effectiveness of the fix: restoration of expected JER |
|
+1 |
| channelData.tsPedestalWidth(2)*channelData.tsPedestalWidth(2)*channelData.tsGain(2)*channelData.tsGain(2) + | ||
| channelData.tsPedestalWidth(3)*channelData.tsPedestalWidth(3)*channelData.tsGain(3)*channelData.tsGain(3)); | ||
| double sipmDarkCurrentWidth2 = 0.; | ||
| if(dcConstraint_) sipmDarkCurrentWidth2 = sipmDarkCurrentWidth*sipmDarkCurrentWidth; |
There was a problem hiding this comment.
IIRC, the dark current distribution is substantially non-Gaussian. So, the constraint done this way is perhaps not the best solution.
If so, I guess it's still OK for a trial for phase-2 studies.
There was a problem hiding this comment.
yes, that will need to be a long-term development item for M2.
|
@kpedro88 |
|
@kpedro88 |
|
|
On 9/11/17 11:16 AM, Kevin Pedro wrote:
@slava77 <https://github.com/slava77>
1. You'll have to ask @igv4321 <https://github.com/igv4321> about
fillDescriptions for HCAL reco code; well beyond the scope of this
PR, and also I'm not even sure if it makes sense with an algorithm
plugin system like the one we have.
OK. This wasn't for this PR, more of a check on the status of this.
There are solutions (check tracking code) where the fillDescriptions was
eventually made feasible.
2. Yes, the default inputs have no aging, so the effect of slightly
reducing the constraint from the third term of M2 is small. The
default dark current is tiny (less than an MeV). If you look at the
presentation I linked in the PR description, you can see the effect
of the fix with aging (slide 9).
I saw the slides had large changes in chi2, but not the jenkins diffs.
Lack of aging by default explains what's happening.
Thanks.
…
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20422 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbgZHsU8DwaBknKkBH1jQ0VM7Bzwiks5shXkYgaJpZM4PQFGl>.
|
|
+1
|
cmsbuild
added
fully-signed
reconstruction-approved
and removed
pending-signatures
reconstruction-pending
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 (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
When SiPM aging is enabled, the dark current (an extra source of electronics noise, separate from the regular pedestal) can increase drastically. The pedestal constraint in HCAL Method 2 reconstruction did not account for this increase, leading to biased energy for some RecHits. Now it is included, but only enabled for Phase2. An additional parameter is added to the Method 2 parameter set, which must eventually be propagated to HLT separately (since M2 parameters are not kept in fillDescriptions).
See the presentation https://indico.cern.ch/event/664043/contributions/2713101/attachments/1519347/2372809/hcal_raddam_phase2_status_sept_6_2017.pdf and this plot that demonstrates improvement in jet energy response:
This PR will be backported to 93X for inclusion in the HGCal TDR release (hopefully).
attn: @abdoulline @mariadalfonso @hatakeyamak