Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GEM-CSC Trigger for Run-3 #34582

Merged
merged 11 commits into from
Jul 23, 2021
Merged

GEM-CSC Trigger for Run-3 #34582

merged 11 commits into from
Jul 23, 2021

Conversation

dildick
Copy link
Contributor

@dildick dildick commented Jul 22, 2021

PR description:

This PR puts in place the missing pieces for the GEM-CSC trigger for Run-3

  • I removed CSCUpgradeMotherboard, and absorbed CSCGEMMotherboardME11 and CSCGEMMotherboardME21 into CSCGEMMotherboard. There are now just two motherboards remaining (CSCMotherboard and CSCGEMMotherboard) down from 6. The old GEM processor (GEMCoPadProcessor) and an obsolete lookup table class CSCUpgradeMotherboardLUT were deleted.
  • This massive reduction in number of classes makes the CSCTriggerPrimitivesBuilder a lot simpler.
  • The GEM-to-CSC matching is now CSC-central, that is: GEM coordinates (pad, roll) are translated into key halfstrip and key wiregroup. It is used to be GEM-central, but there is no latency in the trigger to do these operations. For all coordinate translations, we rely on CSCLUTReader and LUTs in https://github.com/cms-data/L1Trigger-CSCTriggerPrimitives/tree/master/GEMCSC/CoordinateConversion.
  • Matching in a chamber in a BX is first attempted for ALCT-CLCT-GEM, then CLCT-2GEM, then ALCT-2GEM. ALCT-CLCT type LCTs are not rejected if there are no matching GEMs.
  • The GE2/1-ME2/1 integrated local trigger is not supported for D41 or D49 geometries with 8 eta partitions. Only the GE2/1 geometry with 16 eta partitions is supported. I tested it with a D76 geometry which works well.
  • The matching is still based on difference in half-strip. Soon, we'll change that to incorporate the CCLUT slope. The halfstrip windows are set to be >98% efficient for pT > 5 GeV muons. I suppose this could be lowered further to pT > 3 GeV.
  • Quality control tests were expanded for the new GEM-CSC trigger primitives. Though I did delete the QC in CSCMuonPortCard. After all, the QC class is designed to operate on a single (O)TMB, rather than on an entire endcap trigger sector.
  • The GEMInternalCluster class members are properly initialized in the constructor.

Follow-up of #34513, #33974 and #33570.

Documentation is being compiled in a detector note (should be ready by end of the Summer).

PR validation:

Tested the code so far on /RelValSingleMuPt10/CMSSW_11_3_0-113X_mcRun4_realistic_v7_2026D76noPU-v1/GEN-SIM-DIGI-RAW. A few efficiency plots are added below.

Efficiency is ~98% in ME1/1 across the eta range. Efficiency is ~98% in ME2/1 across the eta range, except for near |eta|~1.8 and 2.05 where the high voltage drops. This drop can be mitigated if the number of layers on an ALCT is reduced to 3. However, because the backgrounds in Phase-2 not well understood I am keeping the number of layers to at least 4.

I ran interactively on a few thousand events. I did not see any LogWarnings from the LCTQualityControl class about potentially invalid ALCTs, CLCTs or LCTs.

I'll also check the performance on PU200, but that should be largely unaffected.

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:

FYI @tahuang1991 @rathjd

@dildick
Copy link
Contributor Author

dildick commented Jul 22, 2021

Plots showing efficiency of simulated muon to match to an LCT in ME1/1 or ME2/1. The matching is done like so: sim muon -> CSC simhits -> CSC digis -> CSC ALCT/CLCT -> CSC LCT. At least 3 simhits are required in a chamber (but usually there are 6). Final step of the matching requires either an ALCT-to-LCT or CLCT-to-LCT match. We typically don't require both because (1) ALCT-2GEM and CLCT-2GEM LCTs are missing half the CSC information (2) in cases with 2 ALCT and 2 CLCTs it's possible that the trigger decided a different pairing than what is in the MC truth information.

LCT efficiency (CSC trigger) for 10 GeV prompt muons in ME1/1 without pileup

Screen Shot 2021-07-21 at 8 48 27 PM

LCT efficiency (GEM-CSC trigger) for 10 GeV prompt muons in ME1/1 without pileup

Screen Shot 2021-07-21 at 8 48 20 PM

LCT efficiency (CSC trigger) for 10 GeV prompt muons in ME2/1 without pileup

Screen Shot 2021-07-21 at 8 48 13 PM

LCT efficiency (GEM-CSC trigger) for 10 GeV prompt muons in ME2/1 without pileup

Screen Shot 2021-07-21 at 8 48 52 PM

@dildick
Copy link
Contributor Author

dildick commented Jul 22, 2021

Seems like the only PU200 relval with a decent GE2/1 geometry is /RelValSingleMuPt10/CMSSW_11_2_0_pre9-PU_112X_mcRun4_realistic_v4_2026D66PU200-v1/GEN-SIM-DIGI-RAW. Unfortunately, this is no longer supported in CMSSW_12. I would want at least a geometry with muon detector M9 which means D76 or newer

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34582/24133

  • This PR adds an extra 68KB to repository

  • Found files with invalid states:

    • L1Trigger/CSCTriggerPrimitives/interface/CSCGEMMotherboard.h:

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • L1Trigger/CSCTriggerPrimitives (l1)
  • Validation/MuonCSCDigis (dqm)

@andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @rekovic, @jfernan2, @cecilecaillol, @rvenditti can you please review it and eventually sign? Thanks.
@valuev, @ptcox, @Martin-Grunewald this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy, @perrotta you are the release manager for this.

cms-bot commands are listed here

@dildick
Copy link
Contributor Author

dildick commented Jul 22, 2021

@cecilecaillol After this, I expect another PR to enable the GEM-CSC matching based on the CCLUT slope, and some fine-tuning of the matching parameters. But probably nothing as big as the past few PRs... All the machinery is in place. We have not yet developed the GEM-CSC bending angle assignment in the CorrelatedLCTDigi. Likely we will start working on this next as well.

@cecilecaillol
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f15eca/17078/summary.html
COMMIT: 461c721
CMSSW: CMSSW_12_0_X_2021-07-21-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34582/17078/install.sh to create a dev area with all the needed externals and cmssw changes.

Clang Build

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f15eca/17089/summary.html
COMMIT: c9358f1
CMSSW: CMSSW_12_0_X_2021-07-21-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34582/17089/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

----- Begin Fatal Exception 22-Jul-2021 16:31:34 CEST-----------------------
An exception of category 'StdException' occurred while
   [0] Running EventSetup component DDDetectorESProducer/'
Exception Message:
A std::exception was thrown.
An exception of category 'FileInPathError' occurred.
Exception Message:
edm::FileInPath unable to find file Geometry/VeryForwardData/data/RP_Vertical_Device/2021/v1/RP_Vertical_Device.xml anywhere in the search path.
The search path is defined by: CMSSW_SEARCH_PATH
${CMSSW_SEARCH_PATH} is: /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34582/17089/CMSSW_12_0_X_2021-07-21-2300/poison:/cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34582/17089/CMSSW_12_0_X_2021-07-21-2300/src:/cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34582/17089/CMSSW_12_0_X_2021-07-21-2300/external/slc7_amd64_gcc900/data:/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc900/cms/cmssw/CMSSW_12_0_X_2021-07-21-2300/src:/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc900/cms/cmssw/CMSSW_12_0_X_2021-07-21-2300/external/slc7_amd64_gcc900/data
Current directory is: /data/cmsbld/jenkins/workspace/ib-run-pr-relvals/runTheMatrix-results/11634.911_TTbar_14TeV+2021_DD4hep+TTbar_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA

dd4hep: Error interpreting XML nodes of type <Include/>
dd4hep: Error interpreting XML nodes of type <IncludeSection/>
dd4hep: while parsing /cvmfs/cms-ib.cern.ch/nweek-02690/slc7_amd64_gcc900/cms/cmssw/CMSSW_12_0_X_2021-07-21-2300/src/Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml
dd4hep: with plugin:DD4hep_CompactLoader
----- End Fatal Exception -------------------------------------------------

@dildick
Copy link
Contributor Author

dildick commented Jul 22, 2021

The relval failure is not related to this PR.

@cecilecaillol
Copy link
Contributor

+l1

@jfernan2
Copy link
Contributor

@dildick despite the relval failure is not related to the changes in this PR, I'd like to see a complete check of the PR to get the DQM comparison since none of the trials performed so far concluded, so I am retriggering the tests again

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f15eca/17134/summary.html
COMMIT: c9358f1
CMSSW: CMSSW_12_0_X_2021-07-22-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34582/17134/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: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 39
  • DQMHistoTests: Total histograms compared: 2998564
  • DQMHistoTests: Total failures: 66
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2998475
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 38 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 165 log files, 37 edm output root files, 39 DQM output files
  • TriggerResults: no differences found

@jfernan2
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

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

@dildick
Copy link
Contributor Author

dildick commented Jul 23, 2021

@jfernan2 Thanks for the quick turn-around. There are a few minor changes in the L1T CSC DQM, but that's expected since we enhanced ME1/1 and/or ME2/1 LCTs depending on the era.

@dildick
Copy link
Contributor Author

dildick commented Jul 23, 2021

And there are no differences in Run-1 or Run-2 scenarios. I was not expecting any.

@dildick
Copy link
Contributor Author

dildick commented Jul 23, 2021

In all the performance with the tests look good. That will be it for CSC and GEM-CSC trigger for a while. I have all features for CMSSW_12_0_0_pre5 in. There will be a few updates in late Summer or Fall for GEM-CSC trigger, but nothing major.

@qliphy If you want to sign this PR. Thanks.

@qliphy
Copy link
Contributor

qliphy commented Jul 23, 2021

+1

@cmsbuild cmsbuild merged commit ca5f3a5 into cms-sw:master Jul 23, 2021
@dildick dildick deleted the from-CMSSW_12_0_X_2021-07-18-0000-new-GEM-CSC-matching-classes-v2 branch July 23, 2021 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants