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 out-of-bound issue in call to LUT in GEM-CSC trigger #30564

Merged
merged 4 commits into from Jul 9, 2020
Merged

Fix out-of-bound issue in call to LUT in GEM-CSC trigger #30564

merged 4 commits into from Jul 9, 2020

Conversation

dildick
Copy link
Contributor

@dildick dildick commented Jul 6, 2020

PR description:

This PR should fix a bug reported in the GEM-CSC trigger in CMSSW_11_0_X samples, when a pad number goes out of bounds in the [] operator. I also replaced [] with at() and added a few assert statements. The bug was introduced when the pad numbering in GE1/1 was changed from 1->192 to 0->191, and in GE2/1 from 1->384 to 0->383.

PR validation:

Tested on 4000 events in Run-3 sample /DYToLL_M-50_TuneCP5_14TeV-pythia8/Run3Winter20DRMiniAOD-DRFlatPU30to80_110X_mcRun3_2021_realistic_v6-v2/GEN-SIM-RAW. Also tested with WF 23234.0

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

N/A

FYI @eyigitba @tahuang1991

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 6, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 6, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30564/16801

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 6, 2020

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

It involves the following packages:

L1Trigger/CSCTriggerPrimitives

@cmsbuild, @rekovic, @benkrikler can you please review it and eventually sign? Thanks.
@valuev, @ptcox, @Martin-Grunewald this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

return std::make_pair((getLUT()->get_csc_wg_to_gem_roll(theParity))[alct.getKeyWG()].first,
(getLUT()->get_csc_wg_to_gem_roll(theParity))[alct.getKeyWG()].second);
const auto& mymap(getLUT()->get_csc_wg_to_gem_roll(theParity));
return std::make_pair(mymap.at(alct.getKeyWG()).first, mymap.at(alct.getKeyWG()).second);
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this the same as

Suggested change
return std::make_pair(mymap.at(alct.getKeyWG()).first, mymap.at(alct.getKeyWG()).second);
return mymap.at(alct.getKeyWG());

?

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 8, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 8, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30564/16854

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 8, 2020

Pull request #30564 was updated. @cmsbuild, @rekovic, @benkrikler can you please check and sign again.

@silviodonato
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2020

The tests are being triggered in jenkins.

@silviodonato
Copy link
Contributor

urgent

@cmsbuild cmsbuild added the urgent label Jul 9, 2020
@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2020

+1
Tested at: d08b6a0
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a94ef1/7818/summary.html
CMSSW: CMSSW_11_2_X_2020-07-08-2300
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2020

Comparison job queued.

@silviodonato
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit e74fe08 into cms-sw:master Jul 9, 2020
@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2020

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a94ef1/7818/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2787364
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2787313
  • DQMHistoTests: Total skipped: 50
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 154 log files, 17 edm output root files, 37 DQM output files

@dildick dildick deleted the from-CMSSW_11_2_X_2020-07-06-1100-gem-csc-trigger-bugfix branch July 9, 2020 19:06
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