EgHLT Phase-II showershape fix : 11_3_X

[Sam-Harper]

PR description:

This PR disables the calculation of ECAL showershape variables for superclusters which are not in the ECAL. It sets the showershapes to zero so that by default they pass, this makes things easier to build a trigger as it avoids having to OR the ECAL and HGCAL showershape filters (which likely eventually will be made into a single filter but that is not for today).

No physics changes are expected, the change in behaviour is a crash is prevented for PhaseII

Backports will follow to 11_1 and 11_2

PR validation:

PhaseII E/gamma HLT workflows do not crash.

[fwyzard]

@cmsbuild, please test

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32469/20376

• This PR adds an extra 16KB to repository

[cmsbuild]

A new Pull Request was created by @Sam-Harper (Sam Harper) for master.

It involves the following packages:

RecoEgamma/EgammaHLTProducers

@Martin-Grunewald, @fwyzard can you please review it and eventually sign? Thanks.
@sobhatta, @jainshilpi, @Fedespring, @lgray, @calderona, @HuguesBrun, @afiqaize, @varuns23, @cericeci, @ram1123 this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[silviodonato]

please test

[Martin-Grunewald]

I think the bot gets confused as there are 3 PRs all off the same branch. @smuzaffar

[smuzaffar]

yes, this is now an issue with new bot which uses the git commits to keep track of test statuses. github commits statuses do not belong to PR but them are attached to the git commit. We had senn similar issues with #32356 and #32367 ( see #32365 (comment) ) .
I would suggest to create a separate branch for each PR

[fwyzard]

I would suggest to create a separate branch for each PR

I rather disagree - thar adds extra work for all developments that needs to be backported and could otherwise just be based on a common ancestor.

[Sam-Harper]

Yes I much prefer a common ancestor approach as it saves significant time while ensuring exactly the same thing gets backported

[smuzaffar]

I am afraid this is how github commit statuses work :-( If PRs share the commit then they will share the test results too.
The only thing I can do is to use cms/PR#/arch/testname to distinguish between results

[smuzaffar]

@Sam-Harper , I have update bot to use cms/PR#/arch/test to keep track of PR test status. The new statuses will only be used by new commmits/PRs. In order to enable the new statuses for #32469, #32470 and #32471 , can you please push a new commit (e.g. a comment in code) to EgHLTPhaseIIFixes_1100pre9 branch? This will allow to bot to reset the old statuses and start using new one for these PRs

[Sam-Harper]

no problem will do so no, thank you so much for updating the bot, its very much appreciated that we can keep this functionality as I do think it will save us time in the future. Much appreciated

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32469/20426

• This PR adds an extra 16KB to repository

[cmsbuild]

Pull request #32469 was updated. @Martin-Grunewald, @cmsbuild, @fwyzard can you please check and sign again.

[smuzaffar]

please test
thanks @Sam-Harper

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a94e0a/11729/summary.html
CMSSW: CMSSW_11_3_X_2020-12-15-1100/slc7_amd64_gcc900

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 4 differences found in the comparisons
• DQMHistoTests: Total files compared: 36
• DQMHistoTests: Total histograms compared: 2747284
• DQMHistoTests: Total failures: 7
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2747255
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
• Checked 153 log files, 37 edm output root files, 36 DQM output files

[Martin-Grunewald]

+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 will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[silviodonato]

+1