bsunanda:Run2 hcx124 Correctly provides depth index within HCalSD taking care of many special conditions

[bsunanda]

Correctly provide the depth index within HCalSD taking care of many special conditions - main impact on depth index for Plan1 when testNumbering is not chosen at the SIM phase.
Also adds the tools to test this effort
Should override the earlier PR# 18192 (which is closed)

[cmsbuild]

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

It involves the following packages:

Geometry/HcalCommonData
SimG4CMS/Calo
SimG4Core/Application

@civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel this is something you requested to watch as well.
@Muzaffar, @davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

[bsunanda]

Salivates comments for earlier PR

We need to test it thoroughly:

(1) if it cures the HEP17 non-default SIM option issue described in my slides 21/22 of

https://indico.cern.ch/event/624968/contributions/2534560/attachments/1438163/2215131/CMSSW_Status_April4_2017.pdf

(2) if it doesn't affect default (for 2017 MC) layer-> DetId-depths assignment in DIGI
otherwise it (fix) needs back-porting to 90X for MC production...

[bsunanda]

@cmsbuild Please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18910/console Started: 2017/04/04 16:05

[abdoulline]

I've run comparison/validation with this PR:

(A) -----------------------------------------------------------------------------------------------------------
With this PR for layers -> Depths (real DetIds) assigning (i) in SIM, (ii) in DIGI
This shows that the issue described in the slides 21-22 here:
https://indico.cern.ch/event/624968/contributions/2534560/attachments/1438163/2215360/CMSSW_Status_April4_2017.pdf
is fixed

(B) ------------------------------------------------------------------------------------------------------------
(i) with this PR in 91X vs (ii) 90X without this PR, both with layers -> Depths (real DetIds) assigning in DIGI (default).
This illustrates the fact that there is no impact of this PR on the MC default

(C)--------------------------------------------------------------------------------------------------------------
91X+this PR : comparing HEP17 and HEM17 spectra: difference is small, number of towers is similar

(D)----------------------------------------------------------------------------------------------------------------
Comparison of HCAL towers spectra for HEP17 :
(1) 91X + this PR vs
(2) latest 90X out of the box
Shows there is no impact on default MC mode, this plot is more detailed complementary one to (B) above

[ianna]

@bsunanda - could you, please change the title of this PR to, for example, "Correctly Provide the Depth Index within HCalSD" and append the current one in its description? Thanks.

[cmsbuild]

+1
Tested at: 9fd3061
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18194/18910/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

[abdoulline]

@bsunanda
could you, please, follow Yana's suggestion about the title and also back-port this PR to 90X ?
Ah... now I see PR #18205

[ianna]

+1

@bsunanda - please, do document your future PRs

[cmsbuild]

Pull request #18194 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @davidlange6 can you please check and sign again.

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19135/console Started: 2017/04/12 14:19

[ianna]

+1

[cmsbuild]

+1
Tested at: 9c39446
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18194/19135/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• You potentially added 31 lines to the logs
• Reco comparison results: 1613 differences found in the comparisons
• DQMHistoTests: Total files compared: 23
• DQMHistoTests: Total histograms compared: 1921525
• DQMHistoTests: Total failures: 20648
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 1900704
• DQMHistoTests: Total skipped: 173
• DQMHistoTests: Total Missing objects: 0
• Checked 94 log files, 14 edm output root files, 23 DQM output files

[civanch]

+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. @Muzaffar, @davidlange6, @smuzaffar

[davidlange6]

+1