Phase 2 - HGCal - Adding Energy above noise in RecHit

[felicepantaleo]

This PR adds the information about how many times the energy is above one sigma noise in a particular layer.
This information is useful at clustering level and downstream.

Please refer to the first couple of slides of this talk:
https://cernbox.cern.ch/index.php/s/iPYBFGAGtedNv6X

FYI @malgeri @rovere @cseez

[cmsbuild]

A new Pull Request was created by @felicepantaleo (Felice Pantaleo) for master.

It involves the following packages:

DataFormats/HGCRecHit
RecoLocalCalo/HGCalRecProducers

@perrotta, @cmsbuild, @slava77, @kpedro88, @davidlange6 can you please review it and eventually sign? Thanks.
@vandreev11, @sethzenz, @kpedro88, @lgray, @cseez, @pfs this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

[slava77]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21189/console Started: 2017/07/05 21:06

[cmsbuild]

+1
Tested at: 86d9e65
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19572/21189/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
86d9e65
f194090
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19572/21189/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19572/21189/git-merge-result

[cmsbuild]

Comparison job queued.

[cmsbuild]

Pull request #19572 was updated. @perrotta, @cmsbuild, @slava77, @kpedro88, @davidlange6 can you please check and sign again.

[perrotta]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21397/console Started: 2017/07/12 15:03

[cmsbuild]

+1
Tested at: 4264517
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19572/21397/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
de56eaa
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19572/21397/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19572/21397/git-merge-result

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• You potentially added 3 lines to the logs
• Reco comparison results: 10 differences found in the comparisons
• DQMHistoTests: Total files compared: 23
• DQMHistoTests: Total histograms compared: 2015567
• DQMHistoTests: Total failures: 100
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2015301
• DQMHistoTests: Total skipped: 166
• DQMHistoTests: Total Missing objects: 0
• Checked 93 log files, 14 edm output root files, 23 DQM output files

[perrotta]

@felicepantaleo : this PR indeed do imply some small effect on 2023 outputs, even before the last cosmetics:

JR results differ 1 all_OldVSNew_TTbar14TeV2023D17wf20034p0
JR results differ 11 all_OldVSNew_TTbar14TeV2023D19wf20434p0

see e.g. https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_9_3_X_2017-07-11-2300+19572/21262/validateJR/all_OldVSNew_TTbar14TeV2023D19wf20434p0/

Changes are tiny, but do you have an explanation for them? Just to be sure that nothing leaked in inadvertently

[felicepantaleo]

Ciao @perrotta,
The size of the difference you measure is compatible with the numerical error conversion from double to float that I've done in some parts of the PR.

[perrotta]

Overall, the size of step3.root increases by 0,46% (as in step3 of 20434.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D19 wf)

In detail:

• HGCRecHitsSorted_HGCalRecHit_HGCEERecHits_RECO. 1.08217e+06 151852 +13,7%

• HGCRecHitsSorted_HGCalRecHit_HGCEERecHits_RECO. 1.04727e+06 133520

• HGCRecHitsSorted_HGCalRecHit_HGCHEFRecHits_RECO. 140920 20747.9 +14.8%

• HGCRecHitsSorted_HGCalRecHit_HGCHEFRecHits_RECO. 136387 18063.1

• HGCRecHitsSorted_HGCalRecHit_HGCHEBRecHits_RECO. 10890 2169.5 +10,2%

• HGCRecHitsSorted_HGCalRecHit_HGCHEBRecHits_RECO. 10550.5 1969.1

So, the new member signalOverSigmaNoise_ must be there, and take a non negligible amount of the HGCalRecHit size. What I find strange is that I am not able to see it in the step3 root output:

root [8] Events->Draw("HGCRecHitsSorted_HGCalRecHit_HGCEERecHits_RECO.obj.obj.signalOverSigmaNoise()")
Error in TTreeFormula::DefinedVariable: Unknown method:obj.signalOverSigmaNoise() in HGCRecHit
Error in TTreeFormula::Compile: Bad numerical expression : "HGCRecHitsSorted_HGCalRecHit_HGCEERecHits_RECO.obj.obj.signalOverSigmaNoise()"
Info in TSelectorDraw::AbortProcess: Variable compilation failed: {HGCRecHitsSorted_HGCalRecHit_HGCEERecHits_RECO.obj.obj.signalOverSigmaNoise(),}

What I am doing wrong?

[kpedro88]

@perrotta just a sanity check, did you open the ROOT file after merging this PR and compiling?

[perrotta]

@kpedro88 : that's an extremely good point!
Time to get home and have some rest...
Thank you Kevin

[perrotta]

+1
Tiny effects on the 2023 jenkins outputs are indeed compatible with modified precision in some internal calculation
I was at first a bit surprised of the largish size increase as reported in #19572 (comment) while only adding one extra 8 bit uint data member to the HGCalRecHit's: however, it must be noticed that such a >10% increase in size is in the compressed output, and I expect that a 8 bit object cannot be compressed that much. The relative uncompressed size increase is much lower, and agrees with the expectations

[kpedro88]

+1

[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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

[davidlange6]

+1