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

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

Merged
merged 3 commits into from Apr 22, 2021
Merged

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

merged 3 commits into from Apr 22, 2021

Conversation

dildick
Copy link
Contributor

@dildick dildick commented Apr 21, 2021

PR description:

  1. This PR removes the CLCT-centric matching option in the CSC trigger primitive producer. This would allow LCTs to be constructed with the CLCT BX as the reference BX for LCTs. It should be noted that CLCTs have a poorer time resolution than ALCTs, and they are also more sensitive to pileup.

Between 2005 and 2010, the CSC TP emulator was developed for Run-1 at UCLA. That version remained unchanged throughout Run-1. CMSSW_4_X_Y and CMSSW_5_X_Y all have the same latest commit. You can see in CMSSW_4_1_X that the emulator was written based on CLCT-centric matching.

Between 2010 and 2012 Vadim Khotilovich, (TAMU) did CSC trigger primitive studies with PU140 for the "super-LHC". He changed the matching to ALCT-centric for the upgrade OTMBs to improve the time resolution and mitigate some of the effects at high pileup. See 5c65a14. Since 2014 we have been developing the GEM-CSC integrated local trigger based on the upgrade OTMB with ALCT-centric matching.

In February 2018, it was clarified by Andrew Peck (UCLA) to me that the firmware does (and always has done) ALCT-centric matching. The LCT BX is the ALCT BX, and ALCT+CLCT matches are formed by looking at an ALCT in some central time bin, then looking in the surrounding +/- some number of time bins for a CLCT. You can see this in the firmware here. I updated the emulator in February 2018 to reflect this fact (#22235).

...
// CLCT matched or alct-only
wire alct_pulse  = alct0_pipe_vpf;							// ALCT vpf
wire alct_noclct = alct_pulse   && !clct_window_haslcts;	// ALCT arrived, but there was no CLCT window open
wire clct_match  = alct_pulse   &&  clct_window_haslcts;	// ALCT matches CLCT window, push to mpc on current bx

wire clct_last_win    = clct_last_vpf && !clct_last_tag;	// CLCT reached end of window
wire clct_noalct      = clct_last_win && !alct_pulse;		// No ALCT arrived in window, pushed mpc on last bx
wire clct_noalct_lost = clct_last_win &&  alct_pulse && clct_win_best!=winclosing;// No ALCT arrived in window, lost to mpc contention

// ALCT*CLCT match: alct arrived while there were 1 or more un-tagged clcts in the window
assign clct_tag_me  = clct_match;		// Tag the matching clct
assign clct_tag_win = clct_win_best; 	// But get the one with highest priority
...

Ahead of Run-3 it would be good to finally remove the method from the simulation.

  1. This PR also removes all test constructors in the boards. The test constructors in the ALCT, CLCT and TMB were originally designed for beam tests at UCLA (early to mid 2000s). It does not make sense to maintain these constructors. All testing is now done within the CSC trigger primitive builder framework using MC simulation, and collision/cosmic data.

  2. It also consolidates 3 test configurations into a single one.

PR validation:

Tested on WF 11634.0

11634.0_TTbar_14TeV+2021+TTbar_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED Step4-PASSED  - time date Mon Apr 19 22:43:44 2021-date Mon Apr 19 21:56:54 2021; exit: 0 0 0 0 0
1 1 1 1 1 tests passed, 0 0 0 0 0 failed

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:

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33484/22210

ERROR: Build errors found during clang-tidy run.

Suppressed 489 warnings (488 in non-user code, 1 with check filters).
--
L1Trigger/CSCTriggerPrimitives/interface/CSCBaseboard.h:9:10: error: 'DataFormats/CSCDigi/interface/CSCConstants.h' file not found [clang-diagnostic-error]
#include "DataFormats/CSCDigi/interface/CSCConstants.h"
         ^
Suppressed 500 warnings (499 in non-user code, 1 with check filters).
--
L1Trigger/CSCTriggerPrimitives/interface/CSCBaseboard.h:9:10: error: 'DataFormats/CSCDigi/interface/CSCConstants.h' file not found [clang-diagnostic-error]
#include "DataFormats/CSCDigi/interface/CSCConstants.h"
         ^
Suppressed 498 warnings (497 in non-user code, 1 with check filters).
--
L1Trigger/CSCTriggerPrimitives/interface/CSCBaseboard.h:9:10: error: 'DataFormats/CSCDigi/interface/CSCConstants.h' file not found [clang-diagnostic-error]
#include "DataFormats/CSCDigi/interface/CSCConstants.h"
         ^
Suppressed 498 warnings (497 in non-user code, 1 with check filters).
--
L1Trigger/CSCTriggerPrimitives/interface/CSCBaseboard.h:9:10: error: 'DataFormats/CSCDigi/interface/CSCConstants.h' file not found [clang-diagnostic-error]
#include "DataFormats/CSCDigi/interface/CSCConstants.h"
         ^
Suppressed 489 warnings (488 in non-user code, 1 with check filters).
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:128: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

@dildick
Copy link
Contributor Author

dildick commented Apr 21, 2021

@cecilecaillol While this version merges, there seems to be an issue with CMSSW_12_0_X_2021-04-20-1100 itself. I'll try again later with a newer IB

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33484/22219

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

L1Trigger/CSCTriggerPrimitives

@cmsbuild, @rekovic, @cecilecaillol 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 you are the release manager for this.

cms-bot commands are listed here

@dildick
Copy link
Contributor Author

dildick commented Apr 21, 2021

I rebased on top of CMSSW_12_0_X_2021-04-20-2300, which compiles just fine.

@dildick
Copy link
Contributor Author

dildick commented Apr 21, 2021

@cecilecaillol Can you start the tests, please?

@cecilecaillol
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9d8dda/14389/summary.html
COMMIT: ec7b723
CMSSW: CMSSW_12_0_X_2021-04-20-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33484/14389/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: 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
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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@silviodonato
Copy link
Contributor

+1
rebased version of #33476

@cmsbuild cmsbuild merged commit ca7a979 into cms-sw:master Apr 22, 2021
@dildick dildick deleted the CMSSW_12_0_X_2021-04-19-2300-remove-clct-centric-matching branch April 22, 2021 12:37
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

4 participants