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.

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

Update SiPM parameters and fix a few bugs in hardcode conditions #16526

Merged
merged 6 commits into from Nov 16, 2016

Conversation

kpedro88
Copy link
Contributor

@kpedro88 kpedro88 commented Nov 8, 2016

The major change in this PR is new SiPM parameters (gain, photoelectronsToAnalog, dark current, crosstalk). We were informed about a month ago by the hardware experts that the SiPMs were run with an effective overvoltage of ~4.4V during test beam, but subsequent studies found that an overvoltage of 3V maximized signal to noise. Many SiPM parameters depend on voltage, so they need to be updated from the initial testbeam analysis to get better agreement with expected running conditions.

We were sent some pedestal runs from b904, which I analyzed (with the help of numerous experts) to get an average set of parameters. Slides detailing this analysis can be found here: sipm_condition_analysis.pdf.

I also fixed a few minor bugs:

  1. Way back in Configurable hardcode conditions for HCAL #13861, I dumped run2_mc parameters for HCAL and made per-subdetector averages to use as defaults for the hardcode conditions (for development purposes only). At the time, I didn't realize database pedestal values were in ADC, rather than the fC values assumed by the hardcode conditions. This PR adjusts the average values so they're actually in fC (using functionality from Hcal dump calibrations #16499). Again, this is for development only, and has no impact on any official workflows or results.
  2. Also back in Configurable hardcode conditions for HCAL #13861, I didn't account for depths 3 and 4 for HF when assigning gains, leading to a very minor discrepancy (depth 4 has gain=0.14 instead of 0.135). This has been fixed and will be updated in the database.

@abdoulline, I hope we can update all the database conditions at once...

@lihux25, it might be necessary to recheck the ZS thresholds with the new parameters. I would suggest not rerunning the whole study, but just check the existing "best" threshold you found, and then a slightly higher or lower value (to see how things change). To use the hardcode conditions in your config, merge this PR (in any recent IB) and then:

from SLHCUpgradeSimulations.Configuration.HCalCustoms import load_HcalHardcode
process = load_HcalHardcode(process)
process.es_hardcode.useHEUpgrade = cms.bool(True)
process.es_hardcode.useHFUpgrade = cms.bool(True)

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2016

A new Pull Request was created by @kpedro88 (Kevin Pedro) for CMSSW_8_1_X.

It involves the following packages:

CalibCalorimetry/HcalAlgos
CalibCalorimetry/HcalPlugins
SimCalorimetry/HcalSimProducers

@ghellwig, @civanch, @cerminar, @cmsbuild, @franzoni, @mdhildreth, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @mariadalfonso, @tocheng this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@kpedro88
Copy link
Contributor Author

kpedro88 commented Nov 8, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16267/console

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2016

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 9, 2016

@slava77
Copy link
Contributor

slava77 commented Nov 9, 2016

are there any physics signal response plots to go with this PR?

@abdoulline
Copy link

@mazarkin

OK, lets proceed with 2017 conditions update...

On Tue, 8 Nov 2016, Kevin Pedro wrote:

The major change in this PR is new SiPM parameters (gain, photoelectronsToAnalog, dark current,
crosstalk). We were informed about a month ago by the hardware experts that the SiPMs were run
with an effective overvoltage of ~4.4V during test beam, but subsequent studies found that an
overvoltage of 3V maximized signal to noise. Many SiPM parameters depend on voltage, so they
need to be updated from the initial testbeam analysis to get better agreement with expected
running conditions.

We were sent some pedestal runs from b904, which I analyzed (with the help of numerous experts)
to get an average set of parameters. Slides detailing this analysis can be found here:
sipm_condition_analysis.pdf.

I also fixed a few minor bugs:

  1. Way back in Configurable hardcode conditions for HCAL #13861, I dumped run2_mc parameters for HCAL and made per-subdetector averages
    to use as defaults for the hardcode conditions (for development purposes only). At the time,
    I didn't realize database pedestal values were in ADC, rather than the fC values assumed by
    the hardcode conditions. This PR adjusts the average values so they're actually in fC (using
    functionality from Hcal dump calibrations #16499). Again, this is for development only, and has no impact on any
    official workflows or results.
  2. Also back in Configurable hardcode conditions for HCAL #13861, I didn't account for depths 3 and 4 for HF when assigning gains,
    leading to a very minor discrepancy (depth 4 has gain=0.14 instead of 0.135). This has been
    fixed and will be updated in the database.

@abdoulline, I hope we can update all the database conditions at once...

@lihux25, it might be necessary to recheck the ZS thresholds with the new parameters. I would
suggest not rerunning the whole study, but just check the existing "best" threshold you found,
and then a slightly higher or lower value (to see how things change). To use the hardcode
conditions in your config, merge this PR (in any recent IB) and then:

from SLHCUpgradeSimulations.Configuration.HCalCustoms import load_HcalHardcode
process = load_HcalHardcode(process)
process.es_hardcode.useHEUpgrade = cms.bool(True)
process.es_hardcode.useHFUpgrade = cms.bool(True)


You can view, comment on, or merge this pull request online at:

  #16526

Commit Summary
  • fix hardcode gains for hf upgrade
  • fix legacy hardcode pedestals and widths to be in fC
  • remove unnecessary photoelectronsToAnalog param (now taken from db for SiPMs)
  • update SiPM parameters
  • allow separate dark current for different SiPM types
  • also different crosstalk for large SiPMs

File Changes

  • M CalibCalorimetry/HcalAlgos/interface/HcalHardcodeParameters.h (7)
  • M CalibCalorimetry/HcalAlgos/src/HcalDbHardcode.cc (28)
  • M CalibCalorimetry/HcalAlgos/src/HcalHardcodeParameters.cc (4)
  • M CalibCalorimetry/HcalPlugins/python/Hcal_Conditions_forGlobalTag_cff.py (46)
  • M CalibCalorimetry/HcalPlugins/src/HcalHardcodeCalibrations.cc (14)
  • M SimCalorimetry/HcalSimProducers/python/hcalSimParameters_cfi.py (2)

Patch Links:


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the
thread.[AEx02ssIe58TrQ_CPHiOI7V6ATrC-IiHks5q8PbtgaJpZM4Ks-6H.gif]

@kpedro88
Copy link
Contributor Author

kpedro88 commented Nov 9, 2016

@slava77 the hardcode conditions are used directly for Phase2, so to start we can look at those comparisons, e.g. from D5:

This shows the expected decrease in noise. I'll try to make some kind of 2017 single pion scan or something today.

@slava77
Copy link
Contributor

slava77 commented Nov 9, 2016

On 11/9/16 8:56 AM, Kevin Pedro wrote:

@slava77 https://github.com/slava77 the hardcode conditions are used
directly for Phase2, so to start we can look at those comparisons, e.g.
from D5:

https://camo.githubusercontent.com/7fe1b3ea358b20a9de657baa331eaac04400e45d/687474703a2f2f7777772e6b6a706c616e65742e636f6d2f70722f616c6c5f4f6c6456534e65775f54546261723133546556323032334435776632333232347030635f6c6f6731304842484552656348697473536f727465645f686268657265636f5f5f5245434f5f6f626a5f6f626a5f656e657267792e706e67

What does this plot say about response to real hits?

This shows the expected decrease in noise. I'll try to make some kind of
2017 single pion scan or something today.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16526 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcblyRPwk-5sy-myNuG4XKh9tttWOHks5q8Xy9gaJpZM4Ks-6H.

@kpedro88
Copy link
Contributor Author

kpedro88 commented Nov 9, 2016

To me the new (red) distribution around 1 GeV looks narrower than the old (black) distribution, which I would expect from reduced noise. But as I said, I'll make more plots.

@civanch
Copy link
Contributor

civanch commented Nov 9, 2016

+1

@kpedro88
Copy link
Contributor Author

kpedro88 commented Nov 9, 2016

@mariadalfonso the gain change here may require a retuning of the M2 1pulse/3pulse cut. (Could be worthwhile to make this database-dependent, if we always want to change at 20 GeV, then the corresponding fC value can be calculated automatically.)

@kpedro88
Copy link
Contributor Author

kpedro88 commented Nov 9, 2016

@slava77 I ran 1000 50 GeV pions aimed at the endcap and collected HCAL RecHit energy in a cone of dR < 0.3 around the generated pions (selecting events with ECAL energy < 1 GeV). I observe an improvement in the response and resolution, as expected from the motivation for the new parameters (optimize S/N with lower overvoltage). (Blue fit goes with black histo, magenta fit goes with red histo.)

@slava77
Copy link
Contributor

slava77 commented Nov 9, 2016

On 11/9/16 1:09 PM, Kevin Pedro wrote:

@slava77 https://github.com/slava77 I ran 1000 50 GeV pions aimed at
the endcap and collected HCAL RecHit energy in a cone of dR < 0.3 around
the generated pions (selecting events with ECAL energy < 1 GeV). I
observe an improvement in the response and resolution, as expected from
the motivation for the new parameters (optimize S/N with lower
overvoltage). (Blue fit goes with black histo, magenta fit goes with red
histo.)

Strange
If S/N is better, then there should be less noise hits and the total
energy in the cone should go down.
So, your plot tells me that M2 energy went up by more than 7%
(7% from 49.72/46.5 and "more than" from the reduced noise)

If mean M2 energy is higher by more than 7% then we likely lost the
response quality from
what we had in 15959
http://dalfonso.web.cern.ch/dalfonso/DarkCurrentOnSiPM_PR15959.pdf
slide 5

https://camo.githubusercontent.com/2fd31af230cd5b71573aff3bcea439948476abc2/687474703a2f2f6b6a706c616e65742e636f6d2f70722f6863616c2e706e67


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16526 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcbh1_Y4Mt6kdQA_TNr_nEz-OiaEstks5q8bfggaJpZM4Ks-6H.

@kpedro88
Copy link
Contributor Author

kpedro88 commented Nov 9, 2016

Hm, I see your point about the response. It could be due to the point I realized earlier today, which is that the 1pulse-vs-3pulse cut in M2 needs to be adjusted (but I'm not sure where that cut is made in the code: @mariadalfonso, can you point it out?).

Here is M2 vs M0 before this PR:

and after this PR:

@abdoulline
Copy link

abdoulline commented Nov 9, 2016

We've briefly discussed it in some thread with Maria yesterday
("810pre15_2017 relval followup" thread):

ts4Max = (100.,70000.)
http://cmslxr.fnal.gov/source/RecoLocalCalo/HcalRecProducers/python/HBHEMethod2Parameters_cfi.py#0011

ts4Max[1] usage :
http://cmslxr.fnal.gov/source/RecoLocalCalo/HcalRecAlgos/src/PulseShapeFitOOTPileupCorrection.cc#0567

So to keep it ~20 GeV => ts4Max = (100., 45000.)
On Wed, 9 Nov 2016, Kevin Pedro wrote:

Hm, I see your point about the response. It could be due to the point I realized earlier today,
which is that the 1pulse-vs-3pulse cut in M2 needs to be adjusted (but I'm not sure where that
cut is made in the code: @mariadalfonso, can you point it out?).

Here is M2 vs M0 before this PR:
[687474703a2f2f6b6a706c616e65742e636f6d2f70722f4d325f76735f4d305f6f6c642e706e67]

and after this PR:
[687474703a2f2f6b6a706c616e65742e636f6d2f70722f4d325f76735f4d305f6e65772e706e67]


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the
thread.[AEx02nZUSL5zBVxw2RoLUSZ86ZvFqWaCks5q8c6WgaJpZM4Ks-6H.gif]

@kpedro88
Copy link
Contributor Author

kpedro88 commented Nov 9, 2016

@abdoulline thanks, I reran with these adjustments in my config file:

process.hbheprereco.algorithm.ts4Max = cms.vdouble(100.,44650)
process.hbheprereco.algorithm.pedSigmaSiPM = cms.double(0.000659)

(I could add them to this PR also, but they won't make sense for central WFs until we have tags with the new params, so I'll hold off for now.)

The results went the opposite way we wanted...

But M2 vs M0 looks more like before (though there's some spread at the very low end that could throw off the resolution, should be investigated):

And if we look directly at the hcal/(p-ecal) quantity that @mariadalfonso usually shows, the scale looks okay:

so maybe that's a better measure.

@kpedro88
Copy link
Contributor Author

@mmusich please sign when you get back (corresponding changes already present in the database from #16544...)

@mariadalfonso
Copy link
Contributor

@slava77 @kpedro88
I'm making a separate PR with the M2 adjustment to follow up on this and other little things

@slava77
Copy link
Contributor

slava77 commented Nov 13, 2016

since this one is not approved yet, it may be more practical to have all changes in one go

@kpedro88
Copy link
Contributor Author

@slava77 as I pointed out, the corresponding db conditions are already approved and merged. The changes here are for HCAL development and only directly affect Phase2. I would prefer to get this PR merged and then have a separate PR for M2 tuning.

@kpedro88
Copy link
Contributor Author

@mmusich please sign

@mmusich
Copy link
Contributor

mmusich commented Nov 16, 2016

+1
apologies for delay

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit c768c55 into cms-sw:CMSSW_8_1_X Nov 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants