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

Add new ALCT-CLCT match algorithm for high pt muon showers #40572

Merged

Conversation

tahuang1991
Copy link
Contributor

@tahuang1991 tahuang1991 commented Jan 20, 2023

PR description:

Last year we found that current CSC trigger efficiency drops around 5% per station in the inner ring. The root cause is: when high pT muons showered in the CSC chamber, the cathode hits produced by high pT muon is prefired, 1 or 2BX earlier than nominal case, while anode hits are still in-time. Conventional ALCT(built from anode hits)-CLCT(built from cathode hits) match algorithm would be inefficient as CLCTs from high pT muon are also prefired and CLCTs built from secondary hits are more in-time. Therefore we proposed the upgraded ALCT-CLCT match algorithm for muon showers:

  1. define a local zone around the CLCT key halfstrip to flag the local shower if total hits in the local zone is over threshold
  2. if local shower is valid, then ALCT-CLCT match would pick up earlier CLCT, rather than in-time CLCT. if local shower is not found, then it rolls back to conventional ALCT-CLCT match: ALCT would pick up more in-time CLCT, namely CLCT in the center of match window
  3. local shower definition is configurable: both the shower zone and shower threshold. Right now local shower feature is not enabled in the default configuration
  4. it is NOT included in firmware yet but MC studies with TeV muons showed that new ALCT-CLCT match would improve trigger efficiency when muon showers in the CSC station.

PR validation:

presentation on CSC meeting:
https://indico.cern.ch/event/1197553/contributions/5035258/attachments/2504761/4304563/TaoOTMBStatus_TAMU_20220907_EMTFEff.pdf
https://indico.cern.ch/event/1199888/contributions/5047438/attachments/2508640/4311202/TaoOTMBStatus_TAMU_20220907_EMTFEfffollowup.pdf

presentation on L1 DPG meeting: slide16-19
https://indico.cern.ch/event/1216605/contributions/5117665/attachments/2538591/4369875/Tao_OTMB_L1TDPG_20221031.pdf

1.add CLCT sorting by qual+bending for ALCT-CLCT match
2.add local shower check and switch to new CLCT sorting when sorting by Bx is off and there is at least one
local shower around CLCT in the first half of match window
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40572/33815

  • This PR adds an extra 64KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @tahuang1991 (TaoHuang) for master.

It involves the following packages:

  • L1Trigger/CSCTriggerPrimitives (l1)

@epalencia, @cmsbuild, @cecilecaillol, @rekovic can you please review it and eventually sign? Thanks.
@giovanni-mocellin, @JanFSchulte, @eyigitba, @Martin-Grunewald, @missirol, @ptcox, @valuev 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

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6677d5/30091/summary.html
COMMIT: 3fac2a3
CMSSW: CMSSW_13_0_X_2023-01-19-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40572/30091/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: 9 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3555479
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3555451
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 211 log files, 162 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@tahuang1991
Copy link
Contributor Author

any comments before merging this PR to cmssw?

@cecilecaillol
Copy link
Contributor

+l1

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

@eyigitba
Copy link
Contributor

Hi @tahuang1991 , do I understand correctly that sortClctBx is set to True for now and will be changed when the firmware is also updated?

@@ -49,6 +51,8 @@
tmbPhase2 = tmbPhase1.clone(
# ALCT-CLCT stays at 7 for the moment
matchTrigWindowSize = 7,
#use CLCT sorted by Bx only for ALCT-CLCTmatch for now
sortClctBx = cms.bool(True),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sortClctBx = cms.bool(True),
sortClctBx = True,

Comment on lines 420 to 423
if (totalHits >= localShowerThresh)
localShowerFlag[bx] = true;
else
localShowerFlag[bx] = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (totalHits >= localShowerThresh)
localShowerFlag[bx] = true;
else
localShowerFlag[bx] = false;
localShowerFlag[bx] = totalHits >= localShowerThresh;

Comment on lines 172 to 173
void checkLocalShower(
int zone, const std::vector<int> strip[CSCConstants::NUM_LAYERS][CSCConstants::MAX_NUM_HALF_STRIPS_RUN2_TRIGGER]);
Copy link
Contributor

@perrotta perrotta Jan 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is cheaper to pass the vector by reference:

Suggested change
void checkLocalShower(
int zone, const std::vector<int> strip[CSCConstants::NUM_LAYERS][CSCConstants::MAX_NUM_HALF_STRIPS_RUN2_TRIGGER]);
void checkLocalShower(
int zone, const std::vector<int>& strip[CSCConstants::NUM_LAYERS][CSCConstants::MAX_NUM_HALF_STRIPS_RUN2_TRIGGER]);

(there are other such cases in the rest of the code...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay. I will improve the code with your above three suggestions. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i tried to switch to vector reference in checkLocalShower, but since this vector is used in pulseExtension, the compiling failed... Multiple places in ths CSC trigger emulator used the vector rather reference as argument. I think it is better to do a individual commit to improve this.

@tahuang1991
Copy link
Contributor Author

yes

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40572/33882

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

Pull request #40572 was updated. @epalencia, @cmsbuild, @cecilecaillol, @rekovic can you please check and sign again.

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6677d5/30199/summary.html
COMMIT: d6a1755
CMSSW: CMSSW_13_0_X_2023-01-26-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40572/30199/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: 49
  • DQMHistoTests: Total histograms compared: 3555495
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3555470
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 211 log files, 162 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@cecilecaillol
Copy link
Contributor

+l1

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

@perrotta
Copy link
Contributor

+1

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.

5 participants