Fix the GE2/1 pad producer for 16-partition geometries #35779

dildick · 2021-10-22T02:47:21Z

PR description:

The GE2/1 pad producer for 16-partition geometries was not correctly working.

PR validation:

Tested with D80 geometry on file /store/relval/CMSSW_12_1_0_pre4/RelValTenMuExtendedE_0_200/GEN-SIM-DIGI-RAW/121X_mcRun4_realistic_v5_2026D80noPU-v1/2580000/0021ccff-2964-4ee4-9ad3-c0c81bb0fb83.root

if this PR is a backport please specify the original PR and why you need to backport that PR:

N/A

cmsbuild · 2021-10-22T02:52:53Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35779/26137

This PR adds an extra 12KB to repository

cmsbuild · 2021-10-22T02:53:15Z

A new Pull Request was created by @dildick (Sven Dildick) for master.

It involves the following packages:

L1Trigger/L1TGEM (l1)

@cmsbuild, @rekovic, @cecilecaillol can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @missirol this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

rekovic · 2021-10-26T09:21:33Z

please test

cmsbuild · 2021-10-26T16:42:25Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-813a0b/19950/summary.html
COMMIT: da63a45
CMSSW: CMSSW_12_1_X_2021-10-25-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35779/19950/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: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 41
DQMHistoTests: Total histograms compared: 2797338
DQMHistoTests: Total failures: 48
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2797267
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 40 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 173 log files, 37 edm output root files, 41 DQM output files
TriggerResults: no differences found

dildick · 2021-10-26T20:16:14Z

Results make no sense, because Phase-2 scenarios are not analyzing the GE2/1 clusters. See these lines:

https://github.com/cms-sw/cmssw/blob/master/Validation/MuonGEMDigis/plugins/GEMPadDigiClusterValidation.cc#L204-L206

      // ignore 16-partition GE2/1 pads
      if (gemid.isGE21() and digi->nPartitions() == GEMPadDigiCluster::GE21SplitStrip)
        continue;

Clusters from 16-partition geometries are the default for Phase-2. So this validation module needs to be updated before we can see the effect from fixing the GE2/1 clusters.

dildick · 2021-10-26T20:22:13Z

Rather than ignoring them, the validation code should be update so that it can handle 8-partition GE2/1 clusters and 16-partition GE2/1 clusters.

FYI @jshlee

cmsbuild · 2021-10-26T21:07:46Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35779/26235

This PR adds an extra 24KB to repository

cmsbuild · 2021-10-26T21:08:11Z

Pull request #35779 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @rekovic, @jfernan2, @pmandrik, @cecilecaillol, @pbo0, @rvenditti can you please check and sign again.

dildick · 2021-10-26T21:08:48Z

@jshlee @yeckang GEMPadDigiValidation and GEMPadDigiClusterValidation should be flexible enough to handle both 8 and 16 partition GE2/1 geometries right? In which case I can simply delete

      // ignore 16-partition GE2/1 pads
      if (gemid.isGE21() and digi->nPartitions() == GEMPadDigiCluster::GE21SplitStrip)
        continue;

But GEMDigiMatcher needs an update so it can distinguish between the 8 and 16 partition GE2/1 geometry.

dildick · 2021-10-28T12:19:00Z

@jfernan2 Can you restart the tests? To check if the GE2/1 cluster plots are now filled for 16 partition geometries? Thanks!

perrotta · 2021-10-28T12:28:43Z

please test

cmsbuild · 2021-11-19T17:47:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35779/26766

This PR adds an extra 28KB to repository

cmsbuild · 2021-11-19T17:47:29Z

Pull request #35779 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @rekovic, @jfernan2, @pmandrik, @cecilecaillol, @pbo0, @rvenditti can you please check and sign again.

jfernan2 · 2021-11-22T09:26:23Z

please test

cmsbuild · 2021-11-22T18:46:39Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-813a0b/20664/summary.html
COMMIT: e4fc61b
CMSSW: CMSSW_12_2_X_2021-11-21-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35779/20664/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3247021
DQMHistoTests: Total failures: 119
DQMHistoTests: Total nulls: 210
DQMHistoTests: Total successes: 3246670
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.778 KiB( 41 files compared)
DQMHistoSizes: changed ( 11634.0,... ): -0.016 KiB GEM/Pad
DQMHistoSizes: changed ( 11634.0,... ): -0.016 KiB GEM/PadCluster
DQMHistoSizes: changed ( 23234.0,... ): -0.031 KiB GEM/Pad
DQMHistoSizes: changed ( 23234.0,... ): -0.031 KiB GEM/PadCluster
DQMHistoSizes: changed ( 35034.0,... ): -0.078 KiB GEM/Pad
DQMHistoSizes: changed ( 35034.0,... ): -0.078 KiB GEM/PadCluster
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

jfernan2 · 2021-11-23T08:10:08Z

@dildick as usual, I don't have the expertise to judge if the changes introduced are expected so, if you could give green light to them it is apreciated:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_12_2_X_2021-11-21-2300+813a0b/46968/dqm-histo-comparison-summary.html

Specially because there differences in CSC beyond the changes introduced in GEM code

dildick · 2021-11-23T12:40:16Z

The plots look good. There are differences only for WFs which have a muon geometry >= M8 (https://github.com/cms-sw/cmssw/blob/master/Configuration/Geometry/README.md). The Phase-2 WFs with the older M4 geometry do not change. That makes sense, because the pad producer was not broken for M4. For >=M8 the pad producer and pad cluster producer are now working. In the CSC L1T producer, GEM-CSC integrated local triggers are being produced in ME2/1 OTMBs. The CSC DQM is responding to the OTMBs with GEM-CSC LCTs.

jfernan2 · 2021-11-23T15:48:14Z

+1
Thanks @dildick

dildick · 2021-11-23T16:19:04Z

Thanks

dildick · 2021-11-23T18:23:08Z

@rekovic Can you sign off, please?

dildick · 2021-11-29T12:17:58Z

@rekovic Can you sign, please?

rekovic · 2021-11-30T10:00:55Z

+1

cmsbuild · 2021-11-30T10:01:15Z

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

perrotta · 2021-11-30T10:22:52Z

+1

Observed differences in the DQM outputs being blessed by the detector experts

cmsbuild added this to the CMSSW_12_1_X milestone Oct 22, 2021

cmsbuild added code-checks-pending l1-pending orp-pending pending-signatures tests-pending labels Oct 22, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 22, 2021

cmsbuild added tests-started and removed tests-pending labels Oct 26, 2021

cmsbuild added tests-approved and removed tests-started labels Oct 26, 2021

cmsbuild added code-checks-pending dqm-pending tests-pending and removed tests-approved code-checks-approved labels Oct 26, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 26, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 19, 2021

cmsbuild added tests-started and removed tests-pending labels Nov 22, 2021

cmsbuild added tests-approved and removed tests-started labels Nov 22, 2021

cmsbuild added dqm-approved and removed dqm-pending labels Nov 23, 2021

cmsbuild added fully-signed l1-approved and removed l1-pending pending-signatures labels Nov 30, 2021

cmsbuild added orp-approved and removed orp-pending labels Nov 30, 2021

cmsbuild merged commit d80a1fb into cms-sw:master Nov 30, 2021

dildick deleted the from-CMSSW_12_1_X_2021-10-21-1100-fix-ge21-pad-producer branch November 30, 2021 12:44

cmsbuild mentioned this pull request Nov 30, 2021

(Un)packers for hadronic showers at the uGMT input and output #35627

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the GE2/1 pad producer for 16-partition geometries #35779

Fix the GE2/1 pad producer for 16-partition geometries #35779

dildick commented Oct 22, 2021

cmsbuild commented Oct 22, 2021

cmsbuild commented Oct 22, 2021

rekovic commented Oct 26, 2021

cmsbuild commented Oct 26, 2021

dildick commented Oct 26, 2021

dildick commented Oct 26, 2021

cmsbuild commented Oct 26, 2021

cmsbuild commented Oct 26, 2021

dildick commented Oct 26, 2021

dildick commented Oct 28, 2021

perrotta commented Oct 28, 2021

cmsbuild commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

jfernan2 commented Nov 22, 2021

cmsbuild commented Nov 22, 2021

jfernan2 commented Nov 23, 2021

dildick commented Nov 23, 2021

jfernan2 commented Nov 23, 2021

dildick commented Nov 23, 2021

dildick commented Nov 23, 2021

dildick commented Nov 29, 2021

rekovic commented Nov 30, 2021

cmsbuild commented Nov 30, 2021

perrotta commented Nov 30, 2021

Fix the GE2/1 pad producer for 16-partition geometries #35779

Fix the GE2/1 pad producer for 16-partition geometries #35779

Conversation

dildick commented Oct 22, 2021

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Oct 22, 2021

cmsbuild commented Oct 22, 2021

rekovic commented Oct 26, 2021

cmsbuild commented Oct 26, 2021

Comparison Summary

dildick commented Oct 26, 2021

dildick commented Oct 26, 2021

cmsbuild commented Oct 26, 2021

cmsbuild commented Oct 26, 2021

dildick commented Oct 26, 2021

dildick commented Oct 28, 2021

perrotta commented Oct 28, 2021

cmsbuild commented Nov 19, 2021

cmsbuild commented Nov 19, 2021

jfernan2 commented Nov 22, 2021

cmsbuild commented Nov 22, 2021

Comparison Summary

jfernan2 commented Nov 23, 2021

dildick commented Nov 23, 2021

jfernan2 commented Nov 23, 2021

dildick commented Nov 23, 2021

dildick commented Nov 23, 2021

dildick commented Nov 29, 2021

rekovic commented Nov 30, 2021

cmsbuild commented Nov 30, 2021

perrotta commented Nov 30, 2021