EMTF unpacked muons match uGMT unpacked muons #22090
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22090/3214 |
|
A new Pull Request was created by @abrinke1 for master. It involves the following packages: DataFormats/L1TMuon @nsmith-, @cmsbuild, @thomreis, @rekovic can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
| @@ -2,12 +2,11 @@ | |||
|
|
|||
There was a problem hiding this comment.
Hi @abrinke1 I think there is no need to change this file.
There was a problem hiding this comment.
Why not? Don't we want the EMTF unpacker and emulator to match, wherever possible? I confirmed that L1A_BXN() in EMTFBlockSP.cc matches iEvent.bunchCrossing() in MicroGMTConverter.cc. The offset and truncation is required in both to match how the EMTF firmware fills the track address which is transmitted to the uGMT.
There was a problem hiding this comment.
It looks as if you just changed the formatting in this file.
| @@ -24,6 +25,10 @@ void MicroGMTConverter::convert( | |||
| out_cand.setHwQual ( in_track.GMT_quality() ); | |||
| out_cand.setHwHF ( false ); // EMTF: halo -> 1 | |||
| out_cand.setTFIdentifiers ( sector, tftype ); | |||
| // Truncate to 11 bits and offset by 25 from global event BX in EMTF firmware | |||
There was a problem hiding this comment.
This code does not belong here. Instead it should replace line 85. https://github.com/cms-sw/cmssw/pull/22090/files#diff-0098191652eebd75579da4dd04b68427R85
There was a problem hiding this comment.
I would not put the offset in "convert_all", because "convert" can be called stand-alone, and I would not want to have to perform the truncation and shift every time I call "convert".
There was a problem hiding this comment.
Line 85 is not in convert_all but in this function.
out_cand.setTrackSubAddress(l1t::RegionalMuonCand::kBX , in_track.BX()); is written there at the moment, and this should be replaced because it sets the kBX back to the old value.
|
please abort test |
|
Jenkins tests are aborted. |
|
The code-checks are being triggered in jenkins. |
|
@thomreis all comments addressed. Thanks for the tips, and sorry for the confusion. |
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
|
+1 |
|
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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
cmsbuild
added
fully-signed
l1-approved
and removed
l1-pending
pending-signatures
labels
|
+1 |
|
Hi @abrinke1 |
|
@thomreis if this is needed in 10_0_X for online, please mark it explicitly in the next ORP spreadsheet |
Updates to muon sorting and kBX track address in EMTF unpacker and emulator, to match RegionalMuonCandBxCollection unpacked from uGMT. Allows uGMT-vs-EMTF and EMTF data-vs-emulator comparisons to run properly in the DQM. Small fix, significant improvement to monitoring.
Thanks to @thomreis for testing.