Use CSCConstants in CSC trigger, packer, unpacker and track-finders #33429

dildick · 2021-04-14T04:09:28Z

PR description:

The CSC trigger, packer, unpacker and track-finders are loaded with magic numbers. This PR redefines (some of) them in CSCConstants.h. In particular, the following substitutions were made (where applicable):

1 -> CSCDetId::minLayerId() or CSCConstants::NUM_CFEBS_ME1A_GANGED
3 -> CSCConstants::NUM_CFEBS_ME1A_UNGANGED
4 -> CSCConstants::NUM_CFEBS_ME1B
5 -> CSCConstants::MAX_CFEBS or CSCConstants::NUM_DDUS
6 -> CSCDetId::maxLayerId()
7 -> CSCConstants::MAX_CFEBS_RUN2
8 -> CSCConstants::NUM_DISTRIPS_PER_CFEB
16 -> CSCConstants::NUM_STRIPS_PER_CFEB or CSCConstants::NUM_STRIPS_ME1A_GANGED
32 -> CSCConstants::NUM_HALF_STRIPS_PER_CFEB
48 -> CSCConstants::NUM_STRIPS_ME1A_UNGANGED
64 -> CSCConstants::NUM_STRIPS_ME1B
96 -> CSCConstants::NUM_HALF_STRIPS_ME1A_UNGANGED
128 -> CSCConstants::NUM_HALF_STRIPS_ME1B
MAX_NUM_WIRES - > MAX_NUM_WIREGROUPS because the unit of "theta" in the muon endcap trigger is wiregroup, not wire.

The following statements in CSCComparatorData.cc were encapsulated in class functions:

(strip - 1) * 2 + digi.getComparator() -> CSCComparatorDigi::getHalfStrip()
((strip_ - 1) % 16) / 2; -> CSCComparatorDigi::getDiStrip()
(strip_ - 1) / 16; -> CSCComparatorDigi::getCFEB() or CSCStripDigi::getCFEB()

A few constants related to the (maximum) number of half-strips were clarified, introduced or reconfigured in the CSCConstants file.

MAX_NUM_STRIPS_RUN1
MAX_NUM_STRIPS_RUN2
MAX_NUM_HALF_STRIPS_RUN1
MAX_NUM_HALF_STRIPS_RUN2
MAX_NUM_HALF_STRIPS_RUN1_TRIGGER
MAX_NUM_HALF_STRIPS_RUN2_TRIGGER

Several assert statements were tightened in the packer as well.

The file CSCConstants.h was moved from DataFormats/L1TMuon to DataFormats/CSCDigi to minimize dependencies with the L1TMuon system.

This PR is done in preparation of re-enabling the CFEB packing based on CLCT pre-trigger digis for Run-3 Monte Carlo.

PR validation:

Tested with WF 11634.0. There should be 0 changes with respect to the baseline anywhere.

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

N/A

Before submitting your pull requests, make sure you followed this checklist:

verify that the PR is really intended for the chosen branch
verify that changes follow CMS Naming, Coding, And Style Rules
verify that the PR passes the basic test procedure suggested in the CMSSW PR instructions

@ptcox @barvic

cmsbuild · 2021-04-14T04:17:04Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33429/22093

This PR adds an extra 104KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File L1Trigger/CSCTriggerPrimitives/interface/CSCAnodeLCTProcessor.h modified in PR(s): Hit counters for Run-3 shower CSC trigger (HadronicShowerTrigger-2) #33416
- File L1Trigger/CSCTriggerPrimitives/src/CSCAnodeLCTProcessor.cc modified in PR(s): Hit counters for Run-3 shower CSC trigger (HadronicShowerTrigger-2) #33416
- File L1Trigger/CSCTriggerPrimitives/src/CSCCathodeLCTProcessor.cc modified in PR(s): Hit counters for Run-3 shower CSC trigger (HadronicShowerTrigger-2) #33416

cmsbuild · 2021-04-14T04:17:30Z

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

It involves the following packages:

DataFormats/L1TMuon
EventFilter/CSCRawToDigi
L1Trigger/CSCTriggerPrimitives
Validation/MuonCSCDigis

@perrotta, @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @rekovic, @jfernan2, @slava77, @jpata, @cecilecaillol, @rvenditti can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @barvic, @rovere, @valuev, @ptcox, @thomreis, @dinyar 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

jpata · 2021-04-14T10:36:07Z

@cmsbuild please test

cmsbuild · 2021-04-14T10:49:04Z

-1

Failed Tests: Build
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ed5012/14229/summary.html
COMMIT: c6ef6fe
CMSSW: CMSSW_11_3_X_2021-04-13-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33429/14229/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation error when building:

>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-04-13-2300/src/L1Trigger/CSCTrackFinder/src/CSCTFDTReceiver.cc
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-04-13-2300/src/L1Trigger/CSCTrackFinder/src/CSCTFDTReceiverLUT.cc
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-04-13-2300/src/L1Trigger/CSCTrackFinder/src/CSCTFMuonSorter.cc
>> Compiling  /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-04-13-2300/src/L1Trigger/CSCTrackFinder/src/CSCTFPtLUT.cc
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-04-13-2300/src/L1Trigger/CSCTrackFinder/src/CSCSectorReceiverMiniLUT.cc: In static member function 'static global_eta_data CSCSectorReceiverMiniLUT::calcGlobalEtaMEMini(short unsigned int, short unsigned int, short unsigned int, short unsigned int, unsigned int, bool)':
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-04-13-2300/src/L1Trigger/CSCTrackFinder/src/CSCSectorReceiverMiniLUT.cc:75:61: error: 'MAX_NUM_WIRES' is not a member of 'CSCConstants'
   75 |   if ((tcscid > 0) && (tcscid <= 12) && (WG < CSCConstants::MAX_NUM_WIRES)) {
      |                                                             ^~~~~~~~~~~~~
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_11_3_X_2021-04-13-2300/src/L1Trigger/CSCTrackFinder/src/CSCSectorReceiverMiniLUT.cc:116:89: error: 'MAX_NUM_WIRES' is not a member of 'CSCConstants'
  116 |         << " Value of wire group, " << WG << ", exceeds max allowed, " << CSCConstants::MAX_NUM_WIRES << " +++\n";
      |                                                                                         ^~~~~~~~~~~~~

cmsbuild · 2021-04-14T12:38:34Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33429/22098

This PR adds an extra 108KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File L1Trigger/CSCTriggerPrimitives/interface/CSCAnodeLCTProcessor.h modified in PR(s): Hit counters for Run-3 shower CSC trigger (HadronicShowerTrigger-2) #33416
- File L1Trigger/CSCTriggerPrimitives/src/CSCAnodeLCTProcessor.cc modified in PR(s): Hit counters for Run-3 shower CSC trigger (HadronicShowerTrigger-2) #33416
- File L1Trigger/CSCTriggerPrimitives/src/CSCCathodeLCTProcessor.cc modified in PR(s): Hit counters for Run-3 shower CSC trigger (HadronicShowerTrigger-2) #33416

cmsbuild · 2021-04-14T12:39:01Z

Pull request #33429 was updated. @perrotta, @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @rekovic, @jfernan2, @slava77, @jpata, @cecilecaillol, @rvenditti can you please check and sign again.

perrotta · 2021-04-14T12:39:29Z

@cmsbuild please test

cmsbuild · 2021-04-19T18:29:39Z

Pull request #33429 was updated. @perrotta, @civanch, @kmaeshima, @andrius-k, @mdhildreth, @ErnestaP, @cmsbuild, @rekovic, @jfernan2, @ahmad3213, @slava77, @jpata, @cecilecaillol, @rvenditti can you please check and sign again.

perrotta · 2021-04-19T19:07:54Z

please test

cmsbuild · 2021-04-19T21:51:31Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ed5012/14332/summary.html
COMMIT: 03c686e
CMSSW: CMSSW_12_0_X_2021-04-19-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33429/14332/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS Clang-Tidy warnings: There are Clang-Tidy warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ed5012/14332/llvm-analysis/cmsclangtidy.txt for details.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2877046
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2877023
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

cecilecaillol · 2021-04-20T07:29:02Z

+l1

civanch · 2021-04-20T08:06:07Z

+1

perrotta · 2021-04-20T08:31:56Z

+1

Centralized named constants used instead of the hardcoded numerical values scattered all around CMSSW
Jenkins tests pass and show no differences wrt baseline, as it should

silviodonato · 2021-04-20T08:41:24Z

cc @cms-sw/muon-dpg-l2

jfernan2 · 2021-04-20T08:48:15Z

+1

cmsbuild · 2021-04-20T08:48:37Z

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 · 2021-04-20T15:30:46Z

+1

Use constants from CSCConstants

c6ef6fe

cmsbuild added this to the CMSSW_11_3_X milestone Apr 14, 2021

cmsbuild added code-checks-pending dqm-pending l1-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 14, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 14, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 14, 2021

cmsbuild added tests-rejected and removed tests-started labels Apr 14, 2021

Use constants from CSCConstants

8aa2e7f

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-rejected labels Apr 14, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 14, 2021

cmsbuild removed the tests-pending label Apr 14, 2021

cmsbuild added the code-checks-approved label Apr 19, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 19, 2021

cmsbuild added tests-approved and removed tests-started labels Apr 19, 2021

cmsbuild mentioned this pull request Apr 20, 2021

Remove CLCT-centric matching option in CSC trigger primitives #33476

Closed

cmsbuild added l1-approved and removed l1-pending labels Apr 20, 2021

cmsbuild added simulation-approved and removed simulation-pending labels Apr 20, 2021

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Apr 20, 2021

cmsbuild removed the dqm-pending label Apr 20, 2021

cmsbuild added dqm-approved fully-signed and removed pending-signatures labels Apr 20, 2021

cmsbuild added orp-approved and removed orp-pending labels Apr 20, 2021

cmsbuild merged commit 78c951b into cms-sw:master Apr 20, 2021

dildick deleted the from-CMSSW_11_3_X_2021-04-13-1100-use-csc-constants branch April 20, 2021 23:21

Use CSCConstants in CSC trigger, packer, unpacker and track-finders #33429

Use CSCConstants in CSC trigger, packer, unpacker and track-finders #33429

Uh oh!

Conversation

dildick commented Apr 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description:

PR validation:

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

Uh oh!

cmsbuild commented Apr 14, 2021

Uh oh!

cmsbuild commented Apr 14, 2021

Uh oh!

jpata commented Apr 14, 2021

Uh oh!

cmsbuild commented Apr 14, 2021

Build

Uh oh!

cmsbuild commented Apr 14, 2021

Uh oh!

cmsbuild commented Apr 14, 2021

Uh oh!

perrotta commented Apr 14, 2021

Uh oh!

cmsbuild commented Apr 19, 2021

Uh oh!

perrotta commented Apr 19, 2021

Uh oh!

cmsbuild commented Apr 19, 2021

Comparison Summary

Uh oh!

cecilecaillol commented Apr 20, 2021

Uh oh!

civanch commented Apr 20, 2021

Uh oh!

perrotta commented Apr 20, 2021

Uh oh!

silviodonato commented Apr 20, 2021

Uh oh!

jfernan2 commented Apr 20, 2021

Uh oh!

cmsbuild commented Apr 20, 2021

Uh oh!

silviodonato commented Apr 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

dildick commented Apr 14, 2021 •

edited

Loading