HGCAL: fix interpolation boundaries for ToA

[amartelli]

This PR

• fixes a bug spotted by @adas1994 in the computation of the time-of-arrival in the HGCAL digi
• adds some comments to help the understanding of the code
• fixes in the way in which the PCaloHits are cumulated, to deal with PCaloHIts having exactly the same time stamps (very few cases only observed in high PU)

Quick reminder (how ToA firing works):

• In each Silicon cell, the ToA is fired when the cumulative charge is at threshold (12fC).
• Since a full pulse shape is not available, the single time stamps deposited by G4 in each cell (each with a time and a charge) are used:
  - the time-sorted list of time stamps is considered, and their cumulative charge computed.
  - the 2 time stamps with cumulative charge around the threshold (the one before, and the one after) are picked as reference
  - the ToA is extracted with the linear interpolation between the 2 chosen time-stamps, taking the value at which the interpolation crosses the 12fC threshold.

Explanation of the bug fix:

• The bug is in the interpolation of the cumulated charge vs time, to compute the ToA at threshold,
  and consists in the further cumulation of the already cumulated charges.
• The first cumulation of time-stamps is correct
• The identification of the cells with ToA available is correct
• The bug only involves the identification of the time stamps used to interpolate and extract the ToA

We expect an impact if the delay among time stamps is "big", and this is what happens with PU.
The validation run, comparing the performance between the TDR version (with bug) and this PR version show a negligible impact also at 200PU.

Bug, fix and validation documented in the slides:
https://cernbox.cern.ch/index.php/s/gMcLoFLjHXBpNdi

For reference @rovere

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28777/13444

• This PR adds an extra 20KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

• code-format:
  https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28777/13444/code-format.patch
  e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28777/13444/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28777/13446

• This PR adds an extra 20KB to repository

[cmsbuild]

A new Pull Request was created by @amartelli for master.

It involves the following packages:

SimCalorimetry/HGCalSimProducers

@cmsbuild, @civanch, @kpedro88, @mdhildreth can you please review it and eventually sign? Thanks.
@vandreev11, @sethzenz, @makortel, @kpedro88, @lgray, @cseez, @apsallid, @pfs, @deguio, @hatakeyamak this is something you requested to watch as well.
@davidlange6, @silviodonato, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[civanch]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4330/console Started: 2020/01/23 14:12

[cmsbuild]

+1
Tested at: 583c094
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0ee2f/4330/summary.html
CMSSW: CMSSW_11_1_X_2020-01-23-1100
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[amartelli]

Hi @kpedro88
for me CERNbox works, so in case I attach the slides here
HGCAL_genTOA.pdf

[kpedro88]

+upgrade

[cmsbuild]

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, @silviodonato, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

[silviodonato]

please test 23434.1001

[silviodonato]

@trtomei @fwyzard be aware of this bug in 11_0_X GEN-SIM-DIGI, even if the bug has a "negligible impact also at 200PU"

[silviodonato]

please test workflow 23434.1001

[cmsbuild]

The tests are being triggered in jenkins.
Test Parameters:

• MATRIX_EXTRAS = 23434.1001
  https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4399/console Started: 2020/01/29 09:54
  https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4400/console Started: 2020/01/29 10:25

[cmsbuild]

+1
Tested at: 583c094
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c0ee2f/4400/summary.html
CMSSW: CMSSW_11_1_X_2020-01-28-2300
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

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

• /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-c0ee2f/23434.1001_TTbar_14TeV+RecoFullGlobalPU_TestOldDigi_2026D49PU+HARVESTFullGlobalPU_2026D49PU

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 36 differences found in the comparisons
• DQMHistoTests: Total files compared: 34
• DQMHistoTests: Total histograms compared: 2697090
• DQMHistoTests: Total failures: 171
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2696573
• DQMHistoTests: Total skipped: 346
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
• Checked 147 log files, 16 edm output root files, 34 DQM output files

[silviodonato]

+1

[fwyzard]

On Wed, 29 Jan 2020, 09:54 Silvio Donato, ***@***.***> wrote: @trtomei <https://github.com/trtomei> @fwyzard <https://github.com/fwyzard> be aware of this bug in 11_0_X GEN-SIM-DIGI, even if the bug has a "negligible impact also at 200PU"

Thanks for the heads up. I checked with HGCal DPG, and indeed this should not be an issue for the HLT Phase 2 studies.

…