Fix broken LCTs reported in HLT_L1SingleMu25 [11_1_X] #30914

dildick · 2020-07-25T13:35:41Z

PR description:

An inefficiency was reported in HLT_L1SingleMu25 in CMSSW_11_2_0_pre2, which was not present in CMSSW_11_2_0_pre1 (https://its.cern.ch/jira/browse/CMSLITDPG-903). Investigation showed only ME1/2 LCTs (oddly enough) with invalid pattern numbers. This was fixed with 9354dff.

Further investigation revealed that the CLCT processor readout function was not checking if CLCTs were valid, and that GEM-CSC motherboards do not check if incoming pads are valid. The code was also missing quality control. I added a number of checkValid functions that check the properties for ALCT/CLCT/LCT at various stages of the algorithm. LogErrors are printed in case values go out of bounds.

PR validation:

Tested on 10k events of

/RelValSingleMuFlatPt2To100/CMSSW_11_0_0-PU25ns_110X_mcRun4_realistic_v3_2026D49PU200-v1/GEN-SIM-DIGI-RAW
/RelValSingleMuFlatPt2To100/CMSSW_11_0_0-110X_mcRun4_realistic_v2_2026D49noPU-v1/GEN-SIM-DIGI-RAW
I did not see LogErrors of invalid stubs.

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

Backport of #30909.

cmsbuild · 2020-07-25T13:36:07Z

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

It involves the following packages:

DataFormats/CSCDigi
L1Trigger/CSCCommonTrigger
L1Trigger/CSCTriggerPrimitives

@cmsbuild, @rekovic, @benkrikler, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @ptcox, @valuev, @rovere 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

Backported from Fix broken LCTs reported in HLT_L1SingleMu25 [11_2_X] #30909

civanch · 2020-07-26T06:44:51Z

please test

cmsbuild · 2020-07-26T06:45:15Z

The tests are being triggered in jenkins.

CMSSW_11_1_X_2020-07-25-1100/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/8295/console Started: 2020/07/26 08:45

cmsbuild · 2020-07-26T08:08:26Z

+1
Tested at: 5253476
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b170c8/8295/summary.html
CMSSW: CMSSW_11_1_X_2020-07-25-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-07-26T08:08:29Z

Comparison job queued.

cmsbuild · 2020-07-26T13:50:03Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 3 differences found in the comparisons
DQMHistoTests: Total files compared: 36
DQMHistoTests: Total histograms compared: 2780792
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2780740
DQMHistoTests: Total skipped: 50
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
Checked 152 log files, 16 edm output root files, 36 DQM output files

dildick · 2020-07-26T21:11:13Z

Interesting. I just tested this branch in CMSSW_11_1_X_2020-07-25-1100 on a 11_1_0_pre8 relval sample (in particular root://cmsxrootd-site.fnal.gov//store/relval/CMSSW_11_1_0_pre8/RelValZMM_14/GEN-SIM-DIGI-RAW/111X_mcRun3_2021_realistic_v4-v1/20000/E1B44039-321A-284E-91FB-97CD10F43A48.root).

The number of wiregroups the pretrigger loops on is constrained to [0,47]. Somehow the wiregroup number starts inflating past the bounds of the for-loop, ultimately causing a segfault in the anode processor pretrigger function.

...
%MSG-e CSCAnodeLCTProcessor:   CSCTriggerPrimitivesProducer:simCscTriggerPrimitiveDigis  26-Jul-2020 15:58:09 CDT Run: 1 Event: 754
CSCALCTDigi with invalid wire-group: 799; allowed [0, 48]
%MSG
...
%MSG-e CSCAnodeLCTProcessor:   CSCTriggerPrimitivesProducer:simCscTriggerPrimitiveDigis  26-Jul-2020 15:58:09 CDT Run: 1 Event: 754
CSCALCTDigi with invalid wire-group: 1242; allowed [0, 48]
%MSG
...
%MSG-e CSCAnodeLCTProcessor:   CSCTriggerPrimitivesProducer:simCscTriggerPrimitiveDigis  26-Jul-2020 15:58:09 CDT Run: 1 Event: 754
CSCALCTDigi with invalid wire-group: 1244; allowed [0, 48]
%MSG
...

Not sure what is causing this. The anode pretrigger function did not change recently. I also did not see such weird behavior in the corresponding 11_2_X branch (#30909). In fact, I don't recall ever seeing this.

It seems that - at the very least - I need to add another explicit check that the wiregroup in the for-loop is valid.

dildick · 2020-07-26T23:42:59Z

I noticed that if (lct.getKeyWG() > max_wire) in checkValid should be tightened to if (lct.getKeyWG() >= max_wire). E.g. max_wire returns 48 for ME1/1, but wiregroup numbering starts from 0.

silviodonato · 2020-07-27T10:18:54Z

@rekovic ?

ptcox · 2020-07-27T10:31:34Z

Hi Sven, As I am sure you're aware, all offline geometry counts from 1, so the maximum number of wiregroups in ME1/1 is 48 and counts 1-48. If you want to make a statement like 'wiregroup numbering starts from 0' please make sure it is emphasized that this is in the trigger code only. Otherwise non-experts will get very confused. Regards, Tim

…

Sven Dildick ***@***.***> July 27, 2020 at 01:43 I noticed that |if (lct.getKeyWG() > max_wire)| in |checkValid| should be tightened to |if (lct.getKeyWG() >= max_wire)|. E.g. max_wire returns 48 for ME1/1, but wiregroup numbering starts from 0. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#30914 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABGYLHTV74E23P2P4LUJS7TR5S5Q7ANCNFSM4PHOZ6AA>.

rekovic · 2020-07-27T12:19:16Z

hi @dildick
Regarding your comment #30914 (comment).
Do you see the same on the HLT TDR samples, and does this fix the inefficiency we have reported in [1] ?

Why would you not see this in 11_2_X is surprising to me. Is there anything in the geometry and counting that is different b/w 11_1_X and 11_2_X (@civanch @ptcox) ?

[1]
https://hypernews.cern.ch/HyperNews/CMS/get/L1TriggerUpgrades/385/2.html

dildick · 2020-07-27T14:03:40Z

@rekovic I'll check it now on an HLT TDR sample.

cmsbuild · 2020-07-31T06:35:43Z

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

dildick · 2020-07-31T06:36:17Z

A few LCT efficiency plots (not for publication) from /RelValSingleMuFlatPt2To100/CMSSW_11_0_0-PU25ns_110X_mcRun4_realistic_v3_2026D49PU200-v1/GEN-SIM-DIGI-RAW

dildick · 2020-07-31T06:40:19Z

Efficiencies largely above 90%. Outliers are (1) ME1/2, which does not have upgraded processors or motherboard algorithms, (2) ME2/1 near |eta|~1.7. Despite loosening matching windows in GE2/1, I'm not yet able to recover this inefficiency. Requires more investigation.

dildick · 2020-07-31T07:02:31Z

@tahuang1991 Any idea why the GE2/1-ME2/1 algorithm would loose 20% efficiency near |eta|~1.7?

rekovic · 2020-07-31T07:04:10Z

please test

cmsbuild · 2020-07-31T07:04:36Z

The tests are being triggered in jenkins.

CMSSW_11_1_X_2020-07-30-2300/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/8457/console Started: 2020/07/31 11:35

fwyzard · 2020-07-31T07:05:04Z

unhold

fwyzard · 2020-07-31T07:20:16Z

@dildick thank you very much for the plots

I admit I am not familiar with them... do you have them before the fix ?

dildick · 2020-07-31T07:25:30Z

Previously, the ME3/1 and ME4/1 LCT efficiencies (on a PU0 SingleMu relval sample) looked like this:

rekovic · 2020-07-31T07:30:58Z

@dildick
We had reports of TkMu efficiency of about 20% in the EndCap. Your plots show about 20% impovement in TP efficiency, and that is from 70+ to 90+.

Do you have any plots of change in CSC eta-phi coordinates before your work on the updating the algorithms and now ?
Those should not be affected, right ?

cmsbuild · 2020-07-31T10:53:49Z

+1
Tested at: 6011f7f
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b170c8/8457/summary.html
CMSSW: CMSSW_11_1_X_2020-07-30-2300
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-07-31T10:53:51Z

Comparison job queued.

silviodonato · 2020-07-31T13:45:49Z

urgent

cmsbuild · 2020-07-31T14:20:15Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 36
DQMHistoTests: Total histograms compared: 2780792
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2780737
DQMHistoTests: Total skipped: 50
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
Checked 152 log files, 16 edm output root files, 36 DQM output files

silviodonato · 2020-07-31T18:53:00Z

merge

cmsbuild added this to the CMSSW_11_1_X milestone Jul 25, 2020

cmsbuild added comparison-pending l1-pending orp-pending pending-signatures simulation-pending tests-pending labels Jul 25, 2020

dildick changed the title ~~Fix broken ME1/2 LCTs reported in HLT_L1SingleMu25~~ Fix broken ME1/2 LCTs reported in HLT_L1SingleMu25 [11_1_X] Jul 25, 2020

cmsbuild added tests-started and removed tests-pending labels Jul 26, 2020

cmsbuild added tests-approved and removed tests-started labels Jul 26, 2020

cmsbuild added comparison-available and removed comparison-pending labels Jul 26, 2020

dildick mentioned this pull request Jul 26, 2020

Number of wiregroups is incorrectly overwritten in CMSSW_11_1_X_2020-07-25-1100 #30921

Closed

dildick mentioned this pull request Jul 28, 2020

Fix broken LCTs reported in HLT_L1SingleMu25 [11_2_X] #30909

Merged

cmsbuild removed comparison-available tests-approved labels Jul 28, 2020

Fix half strip stagger

6011f7f

cmsbuild added tests-started and removed tests-pending labels Jul 31, 2020

cmsbuild removed the hold label Jul 31, 2020

cmsbuild added tests-approved and removed tests-started labels Jul 31, 2020

cmsbuild added comparison-available and removed comparison-pending labels Jul 31, 2020

cmsbuild added orp-approved and removed orp-pending labels Jul 31, 2020

cmsbuild merged commit 7ad2e58 into cms-sw:CMSSW_11_1_X Jul 31, 2020

This was referenced Aug 3, 2020

Revert #29562 and use low-quality ALCTs only in ME2/1 #31027

Merged

Revert #29562 and use low-quality ALCTs only in ME2/1 #31028

Merged

dildick deleted the from-CMSSW_11_1_X_2020-07-24-1100-check-valid-lcts-v3 branch August 4, 2020 01:50

dildick mentioned this pull request Sep 30, 2020

Remove option to run on single GEM pads; Simplify CSC trigger configuration #31631

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix broken LCTs reported in HLT_L1SingleMu25 [11_1_X] #30914

Fix broken LCTs reported in HLT_L1SingleMu25 [11_1_X] #30914

dildick commented Jul 25, 2020 •

edited

Loading

cmsbuild commented Jul 25, 2020 •

edited

Loading

civanch commented Jul 26, 2020

cmsbuild commented Jul 26, 2020 •

edited

Loading

cmsbuild commented Jul 26, 2020

cmsbuild commented Jul 26, 2020

cmsbuild commented Jul 26, 2020

dildick commented Jul 26, 2020 •

edited

Loading

dildick commented Jul 26, 2020

silviodonato commented Jul 27, 2020

ptcox commented Jul 27, 2020 via email

rekovic commented Jul 27, 2020

dildick commented Jul 27, 2020

cmsbuild commented Jul 31, 2020

dildick commented Jul 31, 2020

dildick commented Jul 31, 2020 •

edited

Loading

dildick commented Jul 31, 2020

rekovic commented Jul 31, 2020

cmsbuild commented Jul 31, 2020 •

edited

Loading

fwyzard commented Jul 31, 2020

fwyzard commented Jul 31, 2020

dildick commented Jul 31, 2020

rekovic commented Jul 31, 2020

cmsbuild commented Jul 31, 2020

cmsbuild commented Jul 31, 2020

silviodonato commented Jul 31, 2020

cmsbuild commented Jul 31, 2020

silviodonato commented Jul 31, 2020

Fix broken LCTs reported in HLT_L1SingleMu25 [11_1_X] #30914

Fix broken LCTs reported in HLT_L1SingleMu25 [11_1_X] #30914

Conversation

dildick commented Jul 25, 2020 • edited Loading

PR description:

PR validation:

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

cmsbuild commented Jul 25, 2020 • edited Loading

civanch commented Jul 26, 2020

cmsbuild commented Jul 26, 2020 • edited Loading

cmsbuild commented Jul 26, 2020

cmsbuild commented Jul 26, 2020

cmsbuild commented Jul 26, 2020

dildick commented Jul 26, 2020 • edited Loading

dildick commented Jul 26, 2020

silviodonato commented Jul 27, 2020

ptcox commented Jul 27, 2020 via email

rekovic commented Jul 27, 2020

dildick commented Jul 27, 2020

cmsbuild commented Jul 31, 2020

dildick commented Jul 31, 2020

dildick commented Jul 31, 2020 • edited Loading

dildick commented Jul 31, 2020

rekovic commented Jul 31, 2020

cmsbuild commented Jul 31, 2020 • edited Loading

fwyzard commented Jul 31, 2020

fwyzard commented Jul 31, 2020

dildick commented Jul 31, 2020

rekovic commented Jul 31, 2020

cmsbuild commented Jul 31, 2020

cmsbuild commented Jul 31, 2020

silviodonato commented Jul 31, 2020

cmsbuild commented Jul 31, 2020

silviodonato commented Jul 31, 2020

dildick commented Jul 25, 2020 •

edited

Loading

cmsbuild commented Jul 25, 2020 •

edited

Loading

cmsbuild commented Jul 26, 2020 •

edited

Loading

dildick commented Jul 26, 2020 •

edited

Loading

dildick commented Jul 31, 2020 •

edited

Loading

cmsbuild commented Jul 31, 2020 •

edited

Loading