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

HGCAL ToA for overlapping events #20460

Merged
merged 19 commits into from Sep 23, 2017
Merged

Conversation

amartelli
Copy link
Contributor

This PR updates the time-of-arrival computation for HGCAL cells and contains:

  • ToA path independent from charge path: the charge computation is unchanged for both low and high signals
  • ToA fired for signals in-time only: consider the hits with energy deposition that contributes to the in-time time sample (0-25ns) of the charge pulse shape, for example hits that arrive within 25ns-50ns from BX=-1 or in 0-25ns for BX=0...
  • ToA fired when the cumulated charge per cell is above threshold: consider all hits from the main event and pileup events and update the ToA estimate if hits earlier in time are cumulated after in the mixing chain
  • realistic ToA: inject a smearing as a function of S/N of each cell that is derived from the simulation of the electronics

To be effective, a specific configuration has to be used in the DIGI step, as in https://github.com/amartelli/reco-prodtools/blob/updateConfigs_ToA/templates/partGun_GSD_template.py#L11-L21

Workflow tested on few events with 200 pileup.
ToA performance validate with a single particle gun at zero pileup.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20460/627

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/PR-20460/627/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/PR-20460/627/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly (this will soon be required for PRs to be considered)

@slava77
Copy link
Contributor

slava77 commented Sep 11, 2017

@amartelli
please apply a patch from #20460 (comment) to resolve the code-check issues

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 20, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/23116/console Started: 2017/09/20 14:26

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2242 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2649068
  • DQMHistoTests: Total failures: 506
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2648373
  • DQMHistoTests: Total skipped: 189
  • DQMHistoTests: Total Missing objects: 0
  • Checked 107 log files, 14 edm output root files, 26 DQM output files

@kpedro88
Copy link
Contributor

@amartelli I observe minor changes in HGC hits in the comparisons for 2023 workflows (and downstream quantities). I also observe minor changes in ECAL and HCAL hits, presumably due to changes in random number usage in the DIGI step.

Do you have any plots showing expected effects in Phase 2 workflows? (i.e. with more than 10 events)

@amartelli
Copy link
Contributor Author

amartelli commented Sep 21, 2017

Hi @kpedro88 I did a validation with ~1000 events and no pileup, with the configurations in the digitizer changed so to enable all the changes in the PR (as in [1]https://github.com/amartelli/reco-prodtools/blob/updateConfigs_ToA/templates/partGun_GSD_template.py#L11-L21).
With this, I checked that the distribution in time of the recHits for a reconstructed shower was reasonable and compatible with the changes implemented. I also checked that the energy distribution as sumPt of the recHits for reconstructed showers were reasonably unchanged.

In the attached plots you can see an example for time comparison and sumpT, red for 910pre2 default release and blue for 930pre4 with the ToA changes. I judged the difference in the energy as possibly due to the difference in release and it's not a major difference anyway.
The plots refer to pdg130 (K0L) at pT=5GeV in a given bin in eta, looking at reconstructed hits in the HGCAL.
I don't have other kind of comparison for the moment.

Now, coming to your observations. Even without the changes in the configuration mentioned above [1], there is a possible change in the number of calls to a random smearing in the digitizer. Indeed with this PR, considering hard process only, energy deposits late by more than 25ns with charge above the threshold for ToA are not considered (and thus not smeared), while in a default release they are. Could this be enough to explain the difference that you observe?

hgcal_rechits_sumpt_pdg130_pt5gev_eta2 75
hgcal_rechits_timedistrib_pdg130_pt5gev_eta1 75_rad0

@kpedro88
Copy link
Contributor

@amartelli thanks for the check.

I think the calibrations have been changed between 91X and 93X, so that can account for some energy difference.

The change in which hits are smeared would probably cause the calo-related fluctuations I observed.

@kpedro88
Copy link
Contributor

+1

@civanch
Copy link
Contributor

civanch commented Sep 22, 2017

+1

@perrotta
Copy link
Contributor

+1

  • Mostly simulation
  • The only reco related modifications in this PR are in HGCalUncalibRecHitRecWeightsAlgo.h, where jitter (later passed as the "time" of the HGCRecHit) is only saved if the HGCSample is getToAValid(). That flag is set in HGCSample by the HGC simulation (toaFlag in HGCFEElectronics.cc).
  • The distribution of the hits times is validated and shown in HGCAL ToA for overlapping events #20460 (comment)
  • All calo related and derived objects in phase2 jenkins outputs are affected, not only HGC ones (see [1] for example, left here as reference), as already pointed out above. All changes look like random fluctuations, and this is compatible with the modified random sequences in the digitizers, see
    HGCAL ToA for overlapping events #20460 (comment)

[1]
image

@cmsbuild
Copy link
Contributor

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

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit e5bb8c0 into cms-sw:master Sep 23, 2017
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

7 participants