HGCAL: Change ToA to global time in DIGI #39720

stahlleiton · 2022-10-12T21:41:52Z

PR description:

This PR changes the definition of the time of arrival at DIGI level to represent the global time at which the signal is detected in the HGCAL hit (instead of been back-propagated to (0,0,0)). To ensure the time distribution is sampled within 25 ns window, a delay on the global time is added depending on the sub-detector equal to: EE=-9 ns, HEfront=-11 ns, HEback=-14 ns and HFNose=-33 ns.

These time delays are checked in the following plot:

The ToA definition is not changed at RECO level, thus the time of reconstructed HGCAL clusters and tracksters is still the time back-propagated to the center (as currently used), to simplify the review of the DIGI changes. The uncalibrated rechit algorithm takes care of the back-propagation and the removal of the time delay.

This is part of the set of updates already announced in the HGCAL DPG meeting (see https://docs.google.com/presentation/d/11AHfmXCauuNehVb7wZkH5JLHSSI_rbgjducatd0IJwU/edit?usp=sharing).

PR validation:

PR was validated using the following cmsDriver command:
cmsDriver.py TTbar_14TeV_TuneCP5_cfi --conditions auto:phase2_realistic_T21 -n 10 --era Phase2C18I13M9 --eventcontent FEVTDEBUG -s GEN,SIM,DIGI --datatier GEN-SIM-DIGI --beamspot HLLHC14TeV --geometry Extended2026D94 --fileout file:step12_HF.root --customise SLHCUpgradeSimulations/Configuration/aging.customise_aging_3000

The expected changes include:

Minor variations of all CMS digis due to change in number of calls to random number generator engine (correlated per event across all CMSSW).
Simulated HGCAL toa time distribution lowered to 1ns (instead of 5 ns).
Rec hit time distribution peaking at around 0 ns (before was offset to 5 ns).

@pfs @rovere @felicepantaleo

cmsbuild · 2022-10-12T21:49:40Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39720/32554

This PR adds an extra 64KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File SimCalorimetry/HGCalSimProducers/plugins/HGCDigitizer.cc modified in PR(s): Remove a dead assignment of accChargeForToA in the HGCDigitizer #39622

cmsbuild · 2022-10-12T21:50:06Z

A new Pull Request was created by @stahlleiton (Andre Stahl) for master.

It involves the following packages:

HLTrigger/Configuration (hlt)
RecoLocalCalo/HGCalRecAlgos (upgrade, reconstruction)
RecoLocalCalo/HGCalRecProducers (upgrade, reconstruction)
RecoParticleFlow/PFClusterProducer (reconstruction)
SimCalorimetry/HGCalSimProducers (upgrade, simulation)

@Martin-Grunewald, @civanch, @missirol, @clacaputo, @cmsbuild, @AdrianoDee, @srimanob, @mdhildreth, @mandrenguyen can you please review it and eventually sign? Thanks.
@felicepantaleo, @argiro, @Martin-Grunewald, @bsunanda, @pfs, @thomreis, @trtomei, @seemasharmafnal, @youyingli, @mmarionncern, @sethzenz, @apsallid, @silviodonato, @lgray, @missirol, @simonepigazzini, @beaucero, @vandreev11, @rovere, @cseez, @hatakeyamak, @ebrondol, @rchatter, @edjtscott, @lecriste, @sameasy this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

mandrenguyen · 2022-10-12T21:52:10Z

please test

cmsbuild · 2022-10-13T01:57:06Z

-1

Failed Tests: RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b0cd36/28214/summary.html
COMMIT: 0ccdc6f
CMSSW: CMSSW_12_6_X_2022-10-12-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39720/28214/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

20896.0

----- Begin Fatal Exception 13-Oct-2022 00:49:41 CEST-----------------------
An exception of category 'NoProxyException' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'dqmoffline_8_step'
   [2] Prefetching for module PFCandidateAnalyzerDQM/'PFCandAnalyzerDQM'
   [3] Prefetching for module PATPackedCandidateProducer/'packedPFCandidates'
   [4] Prefetching for module PFLinker/'particleFlow'
   [5] Prefetching for module PFCandidateListMerger/'particleFlowTmp'
   [6] Prefetching for module PFProducer/'particleFlowTmpBarrel'
   [7] Prefetching for module PFBlockProducer/'particleFlowBlock'
   [8] Prefetching for module PFElecTkProducer/'pfTrackElec'
   [9] Prefetching for module GsfTrackProducer/'electronGsfTracks'
   [10] Prefetching for module CkfTrackCandidateMaker/'electronCkfTrackCandidates'
   [11] Prefetching for module ElectronSeedMerger/'electronMergedSeeds'
   [12] Prefetching for module ElectronSeedProducer/'ecalDrivenElectronSeeds'
   [13] Prefetching for module HGCalRecHitProducer/'HGCalRecHit'
   [14] Calling method for module HGCalUncalibRecHitProducer/'HGCalUncalibRecHit'
Exception Message:
No data of type "HGCalGeometry" with label "HGCalHFNoseSensitive" in record "IdealGeometryRecord"
 Please add an ESSource or ESProducer to your job which can deliver this data.
----- End Fatal Exception -------------------------------------------------

20900.0

----- Begin Fatal Exception 13-Oct-2022 00:50:14 CEST-----------------------
An exception of category 'NoProxyException' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'dqmoffline_8_step'
   [2] Prefetching for module PFCandidateAnalyzerDQM/'PFCandAnalyzerDQM'
   [3] Prefetching for module PATPackedCandidateProducer/'packedPFCandidates'
   [4] Prefetching for module PFLinker/'particleFlow'
   [5] Prefetching for module PFCandidateListMerger/'particleFlowTmp'
   [6] Prefetching for module PFProducer/'particleFlowTmpBarrel'
   [7] Prefetching for module PFBlockProducer/'particleFlowBlock'
   [8] Prefetching for module PFElecTkProducer/'pfTrackElec'
   [9] Prefetching for module GsfTrackProducer/'electronGsfTracks'
   [10] Prefetching for module CkfTrackCandidateMaker/'electronCkfTrackCandidates'
   [11] Prefetching for module ElectronSeedMerger/'electronMergedSeeds'
   [12] Prefetching for module ElectronSeedProducer/'ecalDrivenElectronSeeds'
   [13] Prefetching for module HGCalRecHitProducer/'HGCalRecHit'
   [14] Calling method for module HGCalUncalibRecHitProducer/'HGCalUncalibRecHit'
Exception Message:
No data of type "HGCalGeometry" with label "HGCalHFNoseSensitive" in record "IdealGeometryRecord"
 Please add an ESSource or ESProducer to your job which can deliver this data.
----- End Fatal Exception -------------------------------------------------

20834.0

----- Begin Fatal Exception 13-Oct-2022 00:53:16 CEST-----------------------
An exception of category 'NoProxyException' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'dqmoffline_8_step'
   [2] Prefetching for module PFCandidateAnalyzerDQM/'PFCandAnalyzerDQM'
   [3] Prefetching for module PATPackedCandidateProducer/'packedPFCandidates'
   [4] Prefetching for module PFLinker/'particleFlow'
   [5] Prefetching for module PFCandidateListMerger/'particleFlowTmp'
   [6] Prefetching for module PFProducer/'particleFlowTmpBarrel'
   [7] Prefetching for module PFBlockProducer/'particleFlowBlock'
   [8] Prefetching for module PFElecTkProducer/'pfTrackElec'
   [9] Prefetching for module GsfTrackProducer/'electronGsfTracks'
   [10] Prefetching for module CkfTrackCandidateMaker/'electronCkfTrackCandidates'
   [11] Prefetching for module ElectronSeedMerger/'electronMergedSeeds'
   [12] Prefetching for module ElectronSeedProducer/'ecalDrivenElectronSeeds'
   [13] Prefetching for module HGCalRecHitProducer/'HGCalRecHit'
   [14] Calling method for module HGCalUncalibRecHitProducer/'HGCalUncalibRecHit'
Exception Message:
No data of type "HGCalGeometry" with label "HGCalHFNoseSensitive" in record "IdealGeometryRecord"
 Please add an ESSource or ESProducer to your job which can deliver this data.
----- End Fatal Exception -------------------------------------------------

Expand to see more relval errors ...

cmsbuild · 2022-10-13T06:15:09Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39720/32556

This PR adds an extra 56KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File SimCalorimetry/HGCalSimProducers/plugins/HGCDigitizer.cc modified in PR(s): Remove a dead assignment of accChargeForToA in the HGCDigitizer #39622

cmsbuild · 2022-10-13T06:15:34Z

Pull request #39720 was updated. @Martin-Grunewald, @civanch, @missirol, @clacaputo, @cmsbuild, @AdrianoDee, @srimanob, @mdhildreth, @mandrenguyen can you please check and sign again.

pfs · 2022-10-13T06:47:34Z

please test

cmsbuild · 2022-10-17T18:49:33Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b0cd36/28304/summary.html
COMMIT: 9e5fdf5
CMSSW: CMSSW_12_6_X_2022-10-17-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39720/28304/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

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

/pool/condor/dir_20150/jenkins/workspace/compare-root-files-short-matrix/data/PR-b0cd36/2500.601_mc126X+TTBarMINIAOD12.6+NANO_mc12.6+HRV_NANO_mc

Summary:

No significant changes to the logs found
Reco comparison results: 5656 differences found in the comparisons
Reco comparison had 6 failed jobs
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3391158
DQMHistoTests: Total failures: 116417
DQMHistoTests: Total nulls: 10
DQMHistoTests: Total successes: 3274709
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 201 log files, 48 edm output root files, 48 DQM output files
TriggerResults: found differences in 1 / 46 workflows

mandrenguyen · 2022-10-17T20:51:47Z

+reconstruction
Code changes look ok, but hgcal and sim experts should clearly have a look as well.
Comparisons appear to be consistent with just changes to the random number generator, as per the description. I agree that this PR will make a subsequent one that changes the reco time-of-arrival easier to benchmark.

missirol · 2022-10-18T06:14:13Z

+hlt

One question, @stahlleiton : (just for my own understanding)

Minor variations of all CMS digis due to change in number of calls to random number generator engine

Why/how does this number of calls change? (I fail to see it from the PR's diff)

civanch · 2022-10-18T08:38:34Z

+1

stahlleiton · 2022-10-18T13:00:41Z

+hlt

One question, @stahlleiton : (just for my own understanding)

Minor variations of all CMS digis due to change in number of calls to random number generator engine

Why/how does this number of calls change? (I fail to see it from the PR's diff)

Since the value of the time-of-arrival (toa) was changed in the digitizer, then the calls to the random engine changes in:

HGCFEElectronics : since toaColl[fireBX] changes and thus the if(toaColl[fireBX] != 0.f) changes when CLHEP::RandGaussQ::shoot(engine,...) is called.
HGCHEbackDigitizer : since totalIniMIPs is 0 for toa outside of [0, 25] ns time window, and CLHEP::RandPoissonQ::shoot(engine, ..) only changes the random generator seed when poisson mean is > 0 (see link), then changing the toa implicetly also changes when the engine is called by RandPoissonQ.

And since the engine is a pointer to a global engine used across all CMSSW and the seed is changed every time the engine is used, then changing the number of calls to the engine produces different pseudo-random results.

rovere · 2022-10-18T16:10:00Z

+1

stahlleiton · 2022-10-18T17:20:08Z

@upgrade-l2 : is there any pending change on this PR for upgrade sign-off?
@srimanob @AdrianoDee

srimanob · 2022-10-19T00:36:34Z

+Upgrade

cmsbuild · 2022-10-19T00:37:00Z

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

perrotta · 2022-10-19T08:51:33Z

+1

cmsbuild added this to the CMSSW_12_6_X milestone Oct 12, 2022

cmsbuild added code-checks-pending hlt-pending orp-pending pending-signatures reconstruction-pending simulation-pending tests-pending upgrade-pending labels Oct 12, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 12, 2022

cmsbuild added tests-started and removed tests-pending labels Oct 12, 2022

cmsbuild added tests-rejected code-checks-pending tests-pending and removed tests-started tests-rejected code-checks-approved labels Oct 13, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 13, 2022

cmsbuild added tests-started and removed tests-pending labels Oct 13, 2022

cmsbuild added tests-approved and removed tests-started labels Oct 17, 2022

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Oct 17, 2022

cmsbuild added hlt-approved and removed hlt-pending labels Oct 18, 2022

cmsbuild added simulation-approved and removed simulation-pending labels Oct 18, 2022

cmsbuild added hgcal-dpg-approved and removed hgcal-dpg-pending labels Oct 18, 2022

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Oct 19, 2022

cmsbuild added orp-approved and removed orp-pending labels Oct 19, 2022

cmsbuild merged commit 9166b25 into cms-sw:master Oct 19, 2022

stahlleiton deleted the HGCAL_DIGI_LocalTime_CMSSW_12_6_0_pre3 branch October 20, 2022 14:52

This was referenced May 4, 2023

HGCAL: Fix HGCHEB isSiFE configuration #41547

Merged

[13_1_X] HGCAL: Fix HGCHEB isSiFE configuration #41563

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HGCAL: Change ToA to global time in DIGI #39720

HGCAL: Change ToA to global time in DIGI #39720

stahlleiton commented Oct 12, 2022 •

edited

cmsbuild commented Oct 12, 2022

cmsbuild commented Oct 12, 2022

mandrenguyen commented Oct 12, 2022

cmsbuild commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

pfs commented Oct 13, 2022

cmsbuild commented Oct 17, 2022

mandrenguyen commented Oct 17, 2022

missirol commented Oct 18, 2022

civanch commented Oct 18, 2022

stahlleiton commented Oct 18, 2022 •

edited

rovere commented Oct 18, 2022

stahlleiton commented Oct 18, 2022 •

edited

srimanob commented Oct 19, 2022

cmsbuild commented Oct 19, 2022

perrotta commented Oct 19, 2022

HGCAL: Change ToA to global time in DIGI #39720

HGCAL: Change ToA to global time in DIGI #39720

Conversation

stahlleiton commented Oct 12, 2022 • edited

PR description:

PR validation:

cmsbuild commented Oct 12, 2022

cmsbuild commented Oct 12, 2022

mandrenguyen commented Oct 12, 2022

cmsbuild commented Oct 13, 2022

RelVals

cmsbuild commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

pfs commented Oct 13, 2022

cmsbuild commented Oct 17, 2022

Comparison Summary

mandrenguyen commented Oct 17, 2022

missirol commented Oct 18, 2022

civanch commented Oct 18, 2022

stahlleiton commented Oct 18, 2022 • edited

rovere commented Oct 18, 2022

stahlleiton commented Oct 18, 2022 • edited

srimanob commented Oct 19, 2022

cmsbuild commented Oct 19, 2022

perrotta commented Oct 19, 2022

stahlleiton commented Oct 12, 2022 •

edited

stahlleiton commented Oct 18, 2022 •

edited

stahlleiton commented Oct 18, 2022 •

edited