Draft: update hlt2 reco with functional MuonID

needs Rec!2501 (closed) and LHCb!3190 (closed)

mentioned in merge request LHCb!3190 (closed)

I made an MR for the change in the Track: LHCb!3190 (closed)

this calls ChargedProtoParticleAddMuonInfo which expects LHCb::MuonPID objects but with the changes in Rec!2501 (closed), the new MuonIDHlt2Alg returns LHCb::Pr::Muon::PIDs instead. So this won't work and it is probably the cause for the segfault you reported by email (although it's hard to tell for sure without seeing the actual error).

While we are not able to create v2 particles directly in the reconstruction, I think we will need either a converter from LHCb::Pr::Muon::PIDs to LHCb::MuonPID or to modify ChargedProtoParticleAddMuonInfo to use LHCb::Pr::Muon::PIDs directly.

However, LHCb::Pr::Muon::PIDs were developed for HLT1 AFAIK and contain less information than LHCb::MuonPID, is this enough for what we need in HLT2?

Looking at this: https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/Event/RecEvent/include/Event/PrMuonPIDs.h#L29

would one not also need some information about the hits / track in the muon system? I know that the v1 MuonPID had such a pointer, but in this class I don't see it anymore.

for my understanding, what were these links used for? In the v2 model we will zip the Muon PID object (whatever it is) to the Tracks, so we have already a one-to-one relation between both, is that enough or we need something else?

Yes, but the track from the reconstruction does not have the muon hits. What I meant was a pointer to the track object in the muon system, such that you can later keep track what hits were actually used for the ID.

Unless of course you would zip both, the muon-system track and the MuonPID object to the rest, but I was assuming this is not the case.

I see, thanks. Then I agree we need to either include the muon track object in the MuonPID object or add it to the zipped object. In both cases I assume this means modifying MuonIDHlt1Alg. Naive question, don't we also need the muon hits in HLT1 to be able to do TISTOSing?

I was also wondering about TISTOSing... I was not sure if you only TISTOS the track (not matter what the muonID says) or you want to TISTOS the track + the muonID. However, given the (possible) non-perfect overlap of HLT1 and HLT2 even for "common" objects, the muonID might be needed.

There is not a "muon-system track" (never was) similar to an LHCb::Track. We don't estimate any momentun or do any type of fitting with the hits. What we had in the past was a link to the track, but as @cmarinbe said the zipping should avoid having the link and we removed it from the new MuonPID to reduce the size of the event. We can add the closest muon hits in the FOI to the MuonPID as those are the ones from where the rest of the variables are extracted and can be helpful for TISTOSing.

Well, this muon-system track (whatever it is) was used at some point for tracking efficiencies, so at least the LHCbIDs of it must have existed.

But if I understand correctly you have a set of hits that you use for the MuonPID? Is this strictly one per layer, or is there at least an upper bound?

I think that this track was not created by the MuonID and this is what Rec/!2496 (merged) is about, right? The used hits for the MuonPID are at most one per layer as in some cases we can skip a hit but still provide a valid MuonPID object (those I will add).

Yes, please, adding the Muon hits used in the MuonPID sounds cheap and useful. I guess this can be done in a separate MR though, AFAIU they are not used by the ChargedProtoParticleAddMuonInfo.

Coming back to the original issue, a temporal solution can be to write a converter from LHCb::Pr::Muon::PIDs + input_tracks to LHCb::MuonPID and use the latter as input to ChargedProtoParticleAddMuonInfo, what do you think @rvazquez ?

How disruptive would be to change directly the input type in ChargedProtoParticleAddMuonInfo to the LHCb::MuonPID? The MuonPID object will be the same between Hlt1 and Hlt2, and in this way we will avoid to come back to this in the future, and also can be prepared if PID information will be ported upfront to the Hlt1 (although not for now).

the main problem I see is that a reference to the LHCb::MuonPID object is added to the ProtoParticles in ChargedProtoParticleAddMuonInfo so the code downstream might rely on this information being present. It also would require to somehow keep track of the links between tracks and LHCb::Pr::Muon::PIDs (probably by order in the containers) and to clean-up some of the information that is not present anymore (or set it to a default value so code downstream doesn't complain).

Since the long term goal is to deprecate ProtoParticles in favour of the v2 event model, I don't think it's worth the effort to update this code to work with LHCb::Pr::Muon::PIDs. But you know these objects better and probably understand better what ChargedProtoParticleAddMuonInfo requires, so if you think it's easier to modify it than to write a transformer, please go ahead.

Actually, if we are already at it: Could LHCb::Pr::Muon::PIDs be renamed / put in a different namespace? The Pr namespace is normally used for pattern recognition in tracking, and not for PID.

Ok, I am writing a Converter from a v2 to a v1 MuonPID getting inspiration from the v2 to v1 track converter. The only difference between the two are the links (actually SmartRefs) to the track and the trackID. I will put some dummy object on it and leave for a separate MR the proper addition of the hits.

I will also change the name / namespace of the MuonPID to something like LHCb::Event::Muon::PIDs

I have added the converter that works for the MuonPIDs. I am able to run the hlt2 reconstruction on ~30 events but then I still get a seg. violation that comes from the track converter which was already present. I attach the interesting part of the seg. violation. hint.txt

Just a quick question: You updated your stack (i.e. rebased your branch)? The v2::Tracks are now v3::Tracks, but looking at your output, I still see: LHCb::v2::Event::Tracks

No, I didn't. I still have my personal version of this change

what's the status of this @rvazquez do you still get the segfault after rebasing?

changed this line in version 3 of the diff

added Configuration Muon hlt2 labels

added 1 commit

• 12ba8dbe - update reco to add MuonPIDs

Compare with previous version

changed the description

mentioned in merge request Rec!2501 (closed)

added 4 commits

• ce0ef798 - update hlt2_muonid configurable with SOA tracks
• 0b880442 - update hlt2 muon sequence
• 665961f0 - simplify MuonID configuration
• e5bd87c4 - update reco to add MuonPIDs

Compare with previous version

added 1 commit

• cc75bc73 - Fixed formatting

Compare with previous version

added 1 commit

• 86eca05a - add make_unique_id_generator

Compare with previous version

added 1 commit

• 4c675eda - Fixed formatting

Compare with previous version

added 1 commit

• efc831c8 - update sequence

Compare with previous version

mentioned in merge request !1141 (merged)

I have created a new MR !1141 (merged) as I got too many problems trying to align with master. The content is the same as in this one. This one can be closed.

Superseded by !1141 (merged)

closed