Pr 112 x Displaced muon in BMTF and uGMT ( unpacker/emulator/packer) #31608
Conversation
The BMTF with the Kalman filter sends inverted track addresses, the uGMT cancel-out unit has been adapted for this. In this commit it has also been configured to use the new mode and pick barrel muons from the KBMTF collection.
The original field with track addresses was filled with incorrect or incomplete values for most TFs. Using static function from RegionalMuonRawDigiTranslator that I factored out in order to generate the raw track address.
May make the interface to get them more clear.
This makes us a bit more consistent with the other fields.
Co-Authored-By: panoskatsoulis <panos.katsoulis@cern.ch>
This should be configured based on eras in the future.
In the MuonUnpacker the wrong overloaded function was being called, this is fixed now. In the process I also rewrote my earlier attempt, it should be more readable now.
Also add displacement information to uGMT intermediate muons and light refactoring for clarity.
Packs correctly for Run-2 2016, Run-2 from 2017 (w/ extrapolated coordiantes), and Run-3
The original field with track addresses was filled with incorrect or incomplete values for most TFs. Using static function from RegionalMuonRawDigiTranslator that I factored out in order to generate the raw track address.
Includes configuration of packer by Era. Modifications still needed for the BMTF setup to make use of this.
Not done nicely, should be cleaned up.
|
@DinayR explained them somehow in #31608 (comment) |
Right, that should be sufficient. An explicit test of reading an old file could be good though, did you try to do that yet? |
I'm currently running that test. I need to re-emulate the trigger as in previous runs the simKBmtfDigis with that field were dropped. |
|
Hi @makortel, I confirm this works now. Reran the kBMTF emulation to create simKBmtfDigis in vanilla CMSSW_11_0_2 and checked in CMSSW_11_2_X_2020-10-11-2300 with this patch applied. Could see the same value for Cheers, |
|
Thanks @dinyar! |
|
+1 |
|
I wanted to ask you about this plot, but I see it was already discussed. I repeat your comment here just to be 100% sure that it has been understood.
|
|
Hi @silviodonato, ok, I'll have a look and will get back to you. Cheers, |
|
Hi @silviodonato, @Dr15Jones, I was trying to reproduce this on lxplus, but am currently struggling to do so. Is there a recipe? Just running Cheers, |
That is the recipe. In IB the stack trace is From which the line hints that it is the reading of old |
|
Compiling static Long_t offset_Onfile_l1tcLcLRegionalMuonCand_m_hwPt2 = oldObj->GetClass()->GetDataMemberOffset("m_hwPt2");and |
|
The function static void read_l1tcLcLRegionalMuonCand_0( char* target, TVirtualObject *oldObj )
{
static Long_t offset_Onfile_l1tcLcLRegionalMuonCand_m_hwPt2 = oldObj->GetClass()->GetDataMemberOffset("m_hwPt2");The |
|
Hi @makortel, Thanks a lot for that, it gave me the hint that of course the RegionalMuonCand dataformat had changed previously (the hwPt2 and hwDXY fields were added sometime in 2018). So I added another version and an iorule that just keeps hwPtUnconstrained 0 for that version in [1]. Could you have a look if that looks reasonable? If it is, should I make a new PR? Related to this: Should I add a similar rule as [2] for hwDXY to guard against the same potential in the future? Cheers, [1] dinyar@9a307c6 |
|
@dinyar I tried to play a bit with the iorule. I found that we get segfaul using
(from https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideCreatingNewProducts#Class_versioning) |
|
@pcanal see earlier comments in this pull request about crashes in IO rules. |
@silviodonato please mention me in te consequent PR I was after this for tomorrow's report on IBs. If this is still present I'd like to mention what was done to be fixed . I'm leaving this for reference https://tinyurl.com/y2a6t6b8 |
|
See #31848 |
|
Thanks @silviodonato. Re-reading @dinyar's #31608 (comment) this makes sense, and I think #31848 is the correct fix. There is no need to set |
More precisely, the value will be what ever it was set to in the default constructor (ROOT doesn't itself zero any fields). |
Thanks for the correction. In this case the default constructor sets the |
|
Hi @silviodonato, @makortel, @Dr15Jones, Thanks a lot for the explanations! This makes sense to me now. Cheers, |
PR description:
11_2_X: L1T Displaced muons: Pt from unconstrainedVtx
PR validation:
if this PR is a backport please specify the original PR and why you need to backport that PR:
Before submitting your pull requests, make sure you followed this checklist: