Fix SiPM pulses

[kpedro88]

This PR is a major step toward resolving the problem of strange output pulses from the SiPM simulation.

Slides detailing my reasoning: http://kjplanet.com/pr/sipm_pulse_study_3.pdf

Slides validating the fix (from @mariadalfonso): http://dalfonso.web.cern.ch/dalfonso/HCAL/oct11/Phase1Jul21_ValidationDIGIFIX.pdf

@slava77 this could have some impact on the performance observed in #15959.

This PR may be updated shortly with an additional change to the RECO pulse shape (still being tested).

[cmsbuild]

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

It involves the following packages:

SimCalorimetry/HcalSimAlgos

@cmsbuild, @civanch, @mdhildreth, @davidlange6 can you please review it and eventually sign? Thanks.
@mariadalfonso 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]

@cmsbuild please test

[cmsbuild]

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

[kpedro88]

@cmsbuild please test

[cmsbuild]

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

[slava77]

tests got stuck apparently.
In my local tests I see changes in HO.

I suppose, this is intended.
Kevin, please clarify.
There are no HO MC plots in the .pdf you have linked, although the change appears to be generic for all SiPMs.

[kpedro88]

Yes, this problem affected anything using the SiPM simulation (apparently for many years), so the fix will alter results for both HO and HE.

[cmsbuild]

+1
Tested at: c02601a
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16180/15686/summary.html

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16180/15686/summary.html

@slava77 comparisons for the following workflows were not done due to missing matrix map:

• 10021.0_TenMuE_0_200+TenMuE_0_200_pythia8_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017
• 10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017
• 10424.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017NewFPix_GenSimFull+DigiFull_2017NewFPix+RecoFull_2017NewFPix+ALCAFull_2017NewFPix+HARVESTFull_2017NewFPix
• 10624.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017HCALdev_GenSimFull+DigiFull_2017HCALdev+RecoFull_2017HCALdev+ALCAFull_2017HCALdev+HARVESTFull_2017HCALdev
• 10824.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017AllNew_GenSimFull+DigiFull_2017AllNew+RecoFull_2017AllNew+ALCAFull_2017AllNew+HARVESTFull_2017AllNew

[slava77]

+1 (informal) from me: this clearly improves things on the reco side. See notes in #15959 (comment)

[civanch]

+1

[cmsbuild]

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]

+1