EMTF May 2020 emulator update #29767
Conversation
|
+1 |
cmsbuild
added
fully-signed
l1-approved
and removed
l1-pending
pending-signatures
labels
|
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 (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
| CSCInput = cms.InputTag('simCscTriggerPrimitiveDigis','MPCSORTED'), | ||
| CSCComparatorInput = cms.InputTag('simMuonCSCDigis','MuonCSCComparatorDigi'), | ||
| RPCInput = cms.InputTag('simMuonRPCDigis'), | ||
| RPCRecHitInput = cms.InputTag('rpcRecHits'), |
There was a problem hiding this comment.
By quick look it seems that this change could have broken a unit test in TauAnalysis/MCEmbeddingTools, see #29897.
There was a problem hiding this comment.
Sorry, I didn't realize this could be an issue. Yes, the rpcRecHits is not needed at the moment, and can be removed. Should I create a new pull request?
There was a problem hiding this comment.
Should I create a new pull request?
Yes, thanks!
There was a problem hiding this comment.
the rpcRecHits is not needed at the moment
On the other hand, if it becomes needed in the future, it needs to be discussed because that would break the test again. (rpcRecHits is part of reconstruction, which makes me wonder why L1 simulation should depend on reconstruction, but I'm probably missing some detail).
There was a problem hiding this comment.
Yes, I agree with what you said:
rpcRecHitsis part of reconstruction, which makes me wonder why L1 simulation should depend on reconstruction
The reason was because I plan to eventually include Phase-2 iRPC hits in the EMTF emulator. According to the suggestion from Brieuc Francois, the clusterization of the iRPC hits is done in the rpcRecHits, so if I wanted to use the iRPC hits, I have to use rpcRecHits. (There is no dedicated L1 producer to do the clusterization). I realized that it was a strange way of doing things, but it did seem to work (at least to me). But now it turned out to be a problem.
I'm not sure what's the best way to move forward (other than to do development in a private CMSSW branch). It seems like the RPC group should develop a L1 producer? But this is not something I could have a say.
There was a problem hiding this comment.
Could the same EDProducer but with a different module label than rpcRecHits work?
It's the re-use of a module label already used in RECO that causes the specific problem in the unit test.
There was a problem hiding this comment.
Oh I see. So using the EDProducer itself is not an issue, but I shouldn't be using/re-using the exact rpcRecHits. I'll do what you suggested in the future. Thanks!
| iConfig.getParameter<edm::InputTag>("CSCComparatorInput"))), | ||
| tokenRPC_(iConsumes.consumes<emtf::RPCTag::digi_collection>(iConfig.getParameter<edm::InputTag>("RPCInput"))), | ||
| tokenRPCRecHit_( | ||
| iConsumes.consumes<emtf::RPCTag::rechit_collection>(iConfig.getParameter<edm::InputTag>("RPCRecHitInput"))), |
There was a problem hiding this comment.
Seems that the tokenRPCRecHit_ is not used so maybe a simple fix would be to remove this parameter and consumes() call?
PR description:
This PR improves the EMTF emulator configuration. In the past few months, several issues had been identified and fixes have been made. These changes have been consolidated in this PR. Right now, the emulator is configured by the FW version. The FW version is supplied by the GlobalTag, specifically the
L1TMuonEndCapParamsRcdrecord. The BDT trees used for pT assignment are also supplied by the GlobalTag, specifically theL1TMuonEndCapForestRcdrecord. These two records must be in sync. Furthermore, the global "era" (e.g.Run2_2016,Run2_2017,Run2_2018) must be in sync with the GTs. When making changes in the future (e.g.Run3_2021), we need to keep all the above in sync to avoid the issues that occurred in the past few months. By default (if "era" is not specified),Run2_2018is used.Two new classes (VersionControl, EMTFSetup) have been added to handle these configuration issues more transparently. In addition, other improvements have been made to facilitate the development of Run 3 pT assignment method.
Notes:
configure_by_fw_version()which was previously found in SectorProcessor.primConvLUThas been removed from the python cfi. The correct PC LUT version should be inferred from the FW version. If necessary to choose a different PC LUT version, do: (i) turn offFWConfig; (ii) setPrimConvVersionin the "fake" condition file (python/fakeEmtfParams_empty_cff.py); (iii) load the fake condition.asserthave been turned off. If necessary to enable the assertion checks, see the comment aboutemtf_assertininterface/DebugTools.h.PtLUTVarCalchave been merged intoPtAssignmentEngineAux2017. This is done to reduce the number of files used for each pt assignment engine.python/fakeEmtfParams_2018_MC_cff.py,python/fakeEmtfParams_2018_data_cff.py). Other fake condition files have been updated.PR validation:
Validated with:
runTheMatrix.py -l limited -i all -j10runTheMatrix.py -w upgrade -l 20434.0,20504.0 -j10Notifications: @abrinke1 @eyigitba @rekovic