Switching to full pulse nonlinearity for SiPMs #18866
Conversation
|
A new Pull Request was created by @igv4321 (Igor Volobouev) for master. It involves the following packages: RecoLocalCalo/HcalRecProducers @perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@igv4321 Thank you. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
set SiPM reco params for cosmics
|
Pull request #18866 was updated. @perrotta, @cmsbuild, @slava77, @davidlange6 can you please check and sign again. |
|
please test |
|
The tests are being triggered in jenkins. |
|
@slava77 there will be a similar effect in both data and MC. The MC nonlinearity simulation does not exactly match the data, but they're close enough. |
|
It became obvious with splashes that the original treatment of SiPM nonlinearity is inconsistent with the nonlinearity calibrations in the lab. The algorithm (with a slightly different implementation) has been tested by Federico, see https://indico.cern.ch/event/640431/contributions/2597635/attachments/1462296/2259023/20170517_-_joint_DPGPhase1_meeting_on_HEP17.pdf And yes, various relevant people are aware of the upcoming change. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
On 5/19/17 1:57 PM, Igor Volobouev wrote:
And yes, various relevant people are aware of the upcoming change.
nobody mentioned anything HCAL related in the OPS meeting at 2PM Friday.
|
|
Comparison is ready Comparison Summary:
|
|
IIUC, this PR is pending discussion in (Wednesday?) HCAL meeting. |
|
@slava77 correct. |
|
Any outcome on the discussion at the HCal meeting? Are there slides that can be linked? |
|
We had discussions on this, and we are pending results of a validation test. The plan is to test the change in this PR with also another PR that should come for sipm+qie11 pulse shape for HEP17 using the splash data with method 2. The sipm+qie11 pulse shape is being measured at low energies and we don't include any energy dependence. If this performs acceptably at very high energies as in splash (where non-linearity effect is most significant), we would like to push this change forward. If everything goes well, we may hear the outcome of this test tomorrow. |
|
In the meantime - MC test with 1 TeV pions. Two samples of 100k each. (1) Pions shot specifically at HEP17 wedge (1 out of 18 in HE+)
(2) pions shot at HE+ randomly a) iphi distribution in HE+ b) ieta distribution (over all iphi's in HE+ )
|
|
hi,
Regions outside HEP17 are not affected. |
|
btw. linearity fix in the legend is this PR |
|
+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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
|
@davidlange6 |
|
I thought its signoff was coming in the reco
Meeting. It's now agreed to have no effect on the pf calibrations?
Cheers,
David
On 1 Jun 2017, at 21:55, Slava Krutelyov <notifications@github.com<mailto:notifications@github.com>> wrote:
@davidlange6<https://github.com/davidlange6>
what was the reason that this PR was not included in CMSSW_9_2_1?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#18866 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEzywxqJWNA7ppNv0t7wqVPqvazzb9P-ks5r_xcAgaJpZM4Ng9xK>.
|
|
Hi, |
|
On 6/1/17 1:07 PM, David Lange wrote:
I thought its signoff was coming in the reco
Meeting.
OK, this is a clear enough reason.
It's now agreed to have no effect on the pf calibrations?
pf calibrations are symmetric in phi and eta sign.
Also, the current approach uses Gaussian fits to derive calibration
parameters.
So, any changes in HEP17 will not be visible in the PF calibrations due
to the 1/36 suppression of its contribution. (even if the response is
wrong by an order of magnitude).
…
Cheers,
David
On 1 Jun 2017, at 21:55, Slava Krutelyov
***@***.******@***.***>> wrote:
@davidlange6<https://github.com/davidlange6>
what was the reason that this PR was not included in CMSSW_9_2_1?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on
GitHub<#18866 (comment)>,
or mute the
thread<https://github.com/notifications/unsubscribe-auth/AEzywxqJWNA7ppNv0t7wqVPqvazzb9P-ks5r_xcAgaJpZM4Ng9xK>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18866 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbl6LzlkqM6zc-FrdgK_JjvJeOxySks5r_xn9gaJpZM4Ng9xK>.
|
|
Let's see if the symmetric calibrations is considered a bug or feature later.... |
Changing TS-by-TS SiPM nonlinearity calculation to estimating the nonlinearity from the charge collected in the 3 time slices (almost full pulse). This treatment is consistent with what was done for calibrating the SiPM non-linearity so far in the lab. While eventually we will need to go back to correcting nonlinearity for individual time slices, we can not do so with the existing measurements and models.
For a detailed description of the problem, see the talks by Jim Hirschauer, Federico De Guio, and Pawel De Barbaro at https://indico.cern.ch/event/640431/
Please test and integrate asap, this fix is needed for the data taking.