Phase2-hgx333B Fix issues of wrong DetID assignment for V17 geometry of HGCal

[bsunanda]

PR description:

Fix issues of wrong DetID assignment for V17 geometry of HGCal

PR validation:

Use the runTheMatrix test workflows and test scripts in Validation/HGCalCOmmonData

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Nothing special

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40281/33332

• This PR adds an extra 40KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File Geometry/HGCalCommonData/src/HGCalDDDConstants.cc modified in PR(s): Phase2-hgx326X Try to address cassette shift abnormality for HGCal geometry #39625

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-40281/33332/code-format.patch
  e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40281/33332/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40281/33333

• This PR adds an extra 40KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File Geometry/HGCalCommonData/src/HGCalDDDConstants.cc modified in PR(s): Phase2-hgx326X Try to address cassette shift abnormality for HGCal geometry #39625

[cmsbuild]

A new Pull Request was created by @bsunanda (Sunanda Banerjee) for master.

It involves the following packages:

• Geometry/HGCalCommonData (geometry, upgrade)
• SimG4CMS/Calo (simulation)
• Validation/HGCalValidation (dqm)

@civanch, @Dr15Jones, @bsunanda, @makortel, @emanueleusai, @ianna, @ahmad3213, @cmsbuild, @AdrianoDee, @srimanob, @jfernan2, @mdhildreth, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@youyingli, @vandreev11, @fabiocos, @lecriste, @sethzenz, @missirol, @felicepantaleo, @rovere, @lgray, @cseez, @apsallid, @pfs, @thomreis, @hatakeyamak, @trtomei, @ebrondol, @beaucero, @slomeo, @simonepigazzini 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

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40281/33334

• This PR adds an extra 40KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File Geometry/HGCalCommonData/src/HGCalDDDConstants.cc modified in PR(s): Phase2-hgx326X Try to address cassette shift abnormality for HGCal geometry #39625

[cmsbuild]

Pull request #40281 was updated. @civanch, @Dr15Jones, @bsunanda, @makortel, @emanueleusai, @ianna, @ahmad3213, @cmsbuild, @AdrianoDee, @srimanob, @jfernan2, @mdhildreth, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please check and sign again.

[bsunanda]

@cmsbuild Please test

[srimanob]

@cmsbuild please test

[missirol]

not clear why Run3 WF with MkFit is sensitive to this PR

It is likely unrelated to this PR, see #39803.

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-71bddf/29600/summary.html
COMMIT: 67ff2eb
CMSSW: CMSSW_13_0_X_2022-12-13-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40281/29600/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 47 differences found in the comparisons
• DQMHistoTests: Total files compared: 50
• DQMHistoTests: Total histograms compared: 3607814
• DQMHistoTests: Total failures: 1251
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3606541
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
• Checked 216 log files, 166 edm output root files, 50 DQM output files
• TriggerResults: no differences found

[srimanob]

The PR looks OK, D92, D93, and D94 workflows run fine. The failure comparison shows up in D92 in HGCAL which is expected that something should change.

However, the failure comparison does not show up in D93 and D94 which are also based on C18 (v17 of HGCAL). I am not sure that in which case:

• technical issue to make reference <== @smuzaffar Do you have some internal if the reference is make, and comparison happen? Or not with 22834.0, 23234.0
• or the PR does not effect C19, C20 (which are based on v17) <== @bsunanda could you please confirm this?

Thanks very much.

[srimanob]

Kindly ping @cms-sw/dqm-l2 to review on DQM, Validation part. Thanks.

[smuzaffar]

technical issue to make reference <== @smuzaffar Do you have some internal if the reference is make, and comparison happen? Or not with 22834.0, 23234.0

@srimanob , baseline ref was built for all workflows, see https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_13_0_X_2022-12-13-1100+71bddf/54652/files/ and all of these workflows have ref. Note that results on https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_13_0_X_2022-12-13-1100+71bddf/54652/validateJR.html are only shown if there are any changes/failures. Looks like DQM comparison did not find any differences for 22834.0, 23234.0

[srimanob]

Thanks @smuzaffar for confirmation.

@bsunanda could you please confirm that the PR introduces the change in C18 (v17 HGCAL) only, but not the variant of it (C19, C20). Thanks.

[emanueleusai]

+1

[bsunanda]

It only affects V17 geometry of HGCal, not the V16 version. C19 is also a v17 geometry version without cell structure defined at G4 Geometry for full wafers. C20 is also a v17 geometry version with added HFNose. All these versions are essentially v17 HGCal geometry.

[srimanob]

It only affects V17 geometry of HGCal, not the V16 version. C19 is also a v17 geometry version without cell structure defined at G4 Geometry for full wafers. C20 is also a v17 geometry version with added HFNose. All these versions are essentially v17 HGCal geometry.

Hi @bsunanda
I think I don't get from your answer. My question is NOT about V16.

My question is on V17 with C19, C20. Do they expect to change with this PR? Because the PR test does not show that they change, only C18 that shows the change. See comment that I tried to clarify is baseline -vs- baseline+PR are done for them, and we confirm that they are done, but no change is seen.

[bsunanda]

The PR takes care of very rare occurrences of hits in some borderline cases of one type of partial wafers. This will include those borderline hits in digitisation and subsequently. Otherwise, the list of valid cells will not change. If such hits do not appear in some simulations, you will not see any difference. That is why regression is not a very clean test to prove any thing. We have made more careful tests for this PR.

[srimanob]

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

[rappoccio]

+1