silence or fix Muon L1T warnings/errors

[kpedro88]

In logs for step2 of phase2 workflows, I noticed this same error repeated a large number of times:

%MSG-w L1T:  L1TMuonEndCapTrackProducer:simEmtfDigis 03-Nov-2017 18:21:20 CET  Run: 1 Event: 7
EMTF RPC format error: tp_ring = 1
%MSG

It looks like it comes from https://github.com/cms-sw/cmssw/blob/master/L1Trigger/L1TMuonEndCap/src/PrimitiveSelection.cc.

Either the underlying problem should be fixed or the warning should be demoted (e.g. to LogDebug). @dildick can you take a look?

[cmsbuild]

A new Issue was created by @kpedro88 Kevin Pedro.

@davidlange6, @Dr15Jones, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[kpedro88]

assign upgrade,l1

[cmsbuild]

New categories assigned: upgrade,l1

@kpedro88,@mulhearn,@rekovic you have been requested to review this Pull request/Issue and eventually sign? Thanks

[dildick]

@abrinke1 FYI

[abrinke1]

Hi @kpedro88 @dildick @rekovic This is an error because the EMTF emulator is currently not designed to include trigger primitives from RPC ring == 1. Such primitives do not exist in the current trigger. Introducing RPC ring 1 primitives into the emulator (with no other changes) will likely produce code crashes or simply bogus results.

If the Phase II code currently includes iRPCs, changes should be made to the EMTF emulator in the cms-l1t-offline:phase2-l1t-integration-CMSSW_9_4_0_pre2 branch (https://github.com/cms-l1t-offline/cmssw/tree/phase2-l1t-integration-CMSSW_9_4_0_pre2). I will let @jiafulow comment on the best change to make, for now:

1) Simply remove iRPC primitives with a cut in src/PrimitiveSelection.cc or somewhere else. 2) Make sure the iRPC primitives can be incorporated into the EMTF track-building sequence seamlessly, add a flag to python/simEmtfDigis_cfi.py to activate this option.

Best, Andrew

[kpedro88]

@abrinke1 thanks for the explanation. I would prefer if the iRPCs could be functional in the Phase 2 trigger emulation (option 2).

[abrinke1]

Certainly, everyone would like that :) It just has to be coded. I'll let @jiafulow comment on the feasibility / timescale. Perhaps this would be a good topic for tomorrow's L1T Phase II software planning meeting: https://indico.cern.ch/event/677987/ Unfortunately I won't be able to attend.

[jiafulow]

Hi @kpedro88 , @abrinke1 ,

The current EMTF emulator is designed based on the firmware; so it doesn't know how to deal with GEM/iRPC hits. There's a Phase 2 EMTF version that is under development, but it's not ready yet. We want to have a first version early next year.

So, depending on the time scale, if you just want this warning to disappear now, we can simply hide iRPC hits from EMTF, as Andrew suggested in option 1. We can also put the under-development version into CMSSW, which would be what Andrew suggested in option 2. It will work but it's just not going to work well. Knowing the recent CMSSW code integration policies, that would probably take months as there are many changes in the codes. Another option is that we simply wait until we finish our development and just make the PR next year.

[kpedro88]

@jiafulow okay, let's hide the iRPC hits in the emulator for now, since it is a known problem. We can leave this issue open until the new emulator is ready.

[jiafulow]

@kpedro88

Ok. The fix should be as simple as the following:

diff --git a/EMTFSubsystemCollector.cc.orig b/EMTFSubsystemCollector.cc
index 8d8c187..aa20274 100644
--- a/EMTFSubsystemCollector.cc.orig
+++ b/EMTFSubsystemCollector.cc
@@ -46,9 +46,10 @@ void EMTFSubsystemCollector::extractPrimitives(
     auto dend = (*chamber).second.second;
     for( ; digi != dend; ++digi ) {
       if ((*chamber).first.region() != 0) {  // 0 is barrel
-        if (!((*chamber).first.station() <= 2 && (*chamber).first.ring() == 3)) {  // do not include RE1/3, RE2/3
-          out.emplace_back((*chamber).first,digi->strip(),(*chamber).first.layer(),digi->bx());
-        }
+        if ((*chamber).first.station() <= 2 && (*chamber).first.ring() == 3)  continue;  // do not include RE1/3, RE2/3
+        if ((*chamber).first.station() >= 3 && (*chamber).first.ring() == 1)  continue;  // do not include RE3/1, RE4/1
+
+        out.emplace_back((*chamber).first,digi->strip(),(*chamber).first.layer(),digi->bx());
       }
     }
   }

Should this go into the 'master' branch, or some special branch for Phase 2?

[kpedro88]

phase 2 development always goes in the master branch now

[jiafulow]

Ok, I'll open a PR to the master branch

[abrinke1]

Really? @rekovic is this correct? What is the point of the cms-l1t-offline:phase2-l1t-integration branch? (https://github.com/cms-l1t-offline/cmssw/tree/phase2-l1t-integration-CMSSW_9_4_0_pre2)

[kpedro88]

@abrinke1 to me, that looks like some private intermediate organizational branch. It is not part of official-cmssw. (Phase2 developments can be backported to older branches if needed for some specific reason, but by default, development happens in the master branch.)

[abrinke1]

Hi @kpedro88 Sorry, I missed the context of this issue originally - I thought you were working on Phase 2 development code. I didn't realize there were Phase 2 workflows already in the standard CMSSW tests. Jia Fu's solution is the correct one for central CMSSW for now. Cheers, Andrew