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

Fix an incorrect (D)CFEB number assignment in the CSC CLCT producer for Run-3 L1T (CCLUT-12) #33254

Merged
merged 4 commits into from
Mar 29, 2021
Merged

Fix an incorrect (D)CFEB number assignment in the CSC CLCT producer for Run-3 L1T (CCLUT-12) #33254

merged 4 commits into from
Mar 29, 2021

Conversation

dildick
Copy link
Contributor

@dildick dildick commented Mar 24, 2021

PR description:

  • It was reported that the Run-3 CCLUT trigger primitive algorithm shows weird behavior for CSC CLCTs when the key half-strip is on the edge of a (D)CFEB (key half-strip is 0 or 31). This PR fixes that. If a CLCT is on the edge, the (D)CFEB number is changed when the CCLUT-corrected half-strip crosses the (D)CFEB boundary.
  • In addition, 3 class members are added to store the number of strips, half-strips and (D)CFEBs for a chamber.
  • I also changed bool noDigis (which is used as a double-negative) to bool hasDigis.

PR validation:

Tested on a 10 GeV prompt muon sample produced with 11_0_X.

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:

@tahuang1991 @msd14

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33254/21730

  • This PR adds an extra 32KB 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

@silviodonato silviodonato changed the title Fix incorrect CFEB assignment (CCLUT-12) [L1Trigger/CSCTriggerPrimitives] Fix incorrect CFEB assignment (CCLUT-12) Mar 24, 2021
@silviodonato
Copy link
Contributor

please test

@silviodonato
Copy link
Contributor

@dildick @cms-sw/l1-l2 have you planned to use this fix in the next MWGR?

@dildick
Copy link
Contributor Author

dildick commented Mar 24, 2021

@silviodonato No, the CCLUT algorithm is still in testing at B904. It has not yet been deployed at P5. No backport required.

@dildick
Copy link
Contributor Author

dildick commented Mar 24, 2021

A few validation plots (not for publication).

Eff_CSCCLCT_comparison_ME11
Eff_CSCCLCT_comparison_ME12
Eff_CSCCLCT_comparison_ME13
Eff_CSCCLCT_comparison_ME21
Eff_CSCCLCT_comparison_ME22
Eff_CSCCLCT_comparison_ME31
Eff_CSCCLCT_comparison_ME32
Eff_CSCCLCT_comparison_ME41
Eff_CSCCLCT_comparison_ME42

Res_CSCCLCT_poscomparison_ME11
Res_CSCCLCT_poscomparison_ME12
Res_CSCCLCT_poscomparison_ME13
Res_CSCCLCT_poscomparison_ME21
Res_CSCCLCT_poscomparison_ME22
Res_CSCCLCT_poscomparison_ME31
Res_CSCCLCT_poscomparison_ME32
Res_CSCCLCT_poscomparison_ME41
Res_CSCCLCT_poscomparison_ME42

@dildick dildick changed the title [L1Trigger/CSCTriggerPrimitives] Fix incorrect CFEB assignment (CCLUT-12) Fix incorrect CFEB assignment (CCLUT-12) Mar 24, 2021
@silviodonato
Copy link
Contributor

@dildick I'm not sure the average CMS user is able to understand which detector are affected from "Fix incorrect CFEB assignment (CCLUT-12)". Please add some words about CSC and L1T. It happens often that people look at the release notes (eg. https://github.com/cms-sw/cmssw/releases/CMSSW_11_3_0_pre5) to find the origin of some changes observed in RelVals.

@dildick dildick changed the title Fix incorrect CFEB assignment (CCLUT-12) Fix an incorrect CFEB number assignment in the CSC CLCT producer for Run-3 (CCLUT-12) Mar 24, 2021
@dildick dildick changed the title Fix an incorrect CFEB number assignment in the CSC CLCT producer for Run-3 (CCLUT-12) Fix an incorrect (D)CFEB number assignment in the CSC CLCT producer for Run-3 (CCLUT-12) Mar 24, 2021
@dildick dildick changed the title Fix an incorrect (D)CFEB number assignment in the CSC CLCT producer for Run-3 (CCLUT-12) Fix an incorrect (D)CFEB number assignment in the CSC CLCT producer for Run-3 L1T (CCLUT-12) Mar 24, 2021
@dildick
Copy link
Contributor Author

dildick commented Mar 24, 2021

@silviodonato Done!

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0ceeef/13735/summary.html
COMMIT: 7ea174d
CMSSW: CMSSW_11_3_X_2021-03-23-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33254/13735/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: 3 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2639935
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2639906
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@dildick
Copy link
Contributor Author

dildick commented Mar 26, 2021

@silviodonato @rekovic Can this PR be merged?

@silviodonato
Copy link
Contributor

silviodonato commented Mar 26, 2021

is there any comment from @cms-sw/l1-l2 ?

@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

@cmsbuild cmsbuild merged commit 2e998e2 into cms-sw:master Mar 29, 2021
@dildick dildick deleted the from-CMSSW_11_3_X_2021-03-21-2300-cclut-fixes branch March 30, 2021 12:10
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.

4 participants