Draft: Create a container for neutral objects
Implements #104. Due to the requirements of the (several) calorimeter reconstruction tools, it is preferred to avoid having a SIMD-type based container for neutral objects. We rely on the use of the zip machinery together with the construction of SIMD-type objects on the fly from the information of the underlying objects.
Used on Rec!2639 (closed) and Moore!1200
(related also to Phys!1024 (closed))
Merge request reports
Activity
added Calo Event model ThOr labels
assigned to @mramospe
- Resolved by Miguel Ramos Pernas
thanks for this @mramospe For my understanding, what is missing to have a first working version?
I am still trying to make the vertex fitter work with neutrals. Even if we are not using the information for the fit, we have to tell the ThOr combiner how to deal with this kind of objects. Once that is done, I will try to create a dummy line selecting
B^0\to K^{*0}\gamma(better to start with photons than neutral pions) and adapt whatever is needed to parse the photons and buildCaloHypothesesWithOrigin
(I will consider changing the name of the container in the future, since now they do not store an origin).Edited by Miguel Ramos PernasThanks for the details. Let me point out a couple of things that are quite particular when making combinations involving neutrals (we probably discussed this already, but just to be sure it's written somewhere):
- for the standard case we have a decay vertex from where >= 2 tracks and a photon originate (eg B -> Kst (K pi) gamma). In this case, we ideally want the vertex fit to use the information from the tracks only but also that the photon momentum is updated by changing its origin to the position of the vertex, and the B mass is computed from this updated photon momentum.
- there are more complicated cases, where one cannot actually reconstruct the decay vertex, eg Lb -> Lambda gamma, where the Lambda flies before decaying to p pi. In this case, we don't want to run a vertex fit, but just a mass combiner that uses the original photon momentum and assumes the parent originates at the associated PV.
I'd suggest to build all these features step by step, so we have a basic solution soon and keep adding features in further iterations, but to keep in mind when designing the solution what we will need in the end.
Thanks for writing it here
. I definitely remember these cases. The main problem of altering the values of the photons is that we would need to somehow write a new particle (create a copy of the input photon with a corrected value of the direction of flight) and save it somewhere else. It is not so clear to me how to do this right now. At the moment I am stuck with the vertex fitter, which can be relatively-easy adapted to modify the momenta of the parent particle by returning appropriate values of the energy and flight direction (assuming that it comes from the PV at the moment). My main concern now is that it seems that the ThOr
combiner is using theScatterGather
proxy behaviour, which is not currently supported by my accessor by function call due to the fact that I haven't found so far a way to extract the contents of anint_v
(SIMD type) to an array of integers (which I can use directly in the function call as indices).https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/Kernel/LHCbMath/include/LHCbMath/SIMDWrapper.h#L179 <- does that not work for you?
@graven @cmarinbe, the current implementation of
Moore
forChargedBasics
objects seems to use aProto2ChargedBasics
converter, so I was wondering if we should do something similar with neutrals or whether we could create them straight away fromLHCb::Event::Calo::v2::Hypotheses
. Are we currently creating (or is there a way to create)LHCb::Event::Calo::v2::Hypotheses
in the reconstruction sequence ofMoore
? Is there also a way to convert the information from aLHCb::ProtoParticle
and store it in theHypotheses
container? Maybe we can go straight for the first option, but I we might want consider whether we should validate both methods or not.If we can skip making
ProtoParticle
s, then we should ;-) And yes, Moore createsLHCb::Event::Calo::v2::Hypotheses
, if you eg. dogit grep Hypo | grep -i operator
in Rec, then you'll findCaloFutureMergedPi0
andClassifyPhotonElectronAlg
so with those two you getHypotheses
for photons, electrons and merged pi0.and perhaps we should open an issue to make ChargedBasic
directly fromv3::Track
(orv1::Track
for now) instead of going throughProtoParticle
...Edited by Gerhard Raven@graven could you point me to the place in
Moore
where theLHCb::Event::Calo::v2::Hypotheses
objects are created? So far what I see inHlt/RecoConf/python/RecoConf/calorimeter_reconstruction.py
seems to be allv1
. There are even somev2-to-v1
converters but they are not used.You're looking at the right thing, but your conclusion isn't quite right -- yes, in the end it returns 'everything v1', but to make 'everything v1' it does so by first making 'everything v2' and then converting back. So I don't think your statement that the converters are not used is correct!
To be specific, lets consider eg. the returned 'photons' which is defined as:
"photons": old_photons,
but then:
old_photons = HypoConverter( InputHypos=photons, InputClusters=old_ecalClusters).OutputHypos
where 'InputHypos=photons" means this is using:
PhElOutput = make_photons_and_electrons(best_tracks, ecalClusters, pvs) photons = PhElOutput["photons"]
and then going to
make_photons_and_electrons
, and there you'll seephotons = photonElectronAlg.OutputPhotons
with
photonElectronAlg = ClassifyPhotonElectronAlg( ... )
where
ClassifyPhotonElectronAlg
is this bit of code which here creates the v2Hypotheses
for both electrons and photons (without ever mentioning v1).So the fact that make_calo returns a dictionary with v1 objects does not mean that the v2 ones are not made -- it is just for backward compatibility that it (still) returns v1 hypos, but they are all made by converting v2 Hypotheses -- which is also what you see in the flame graphs of Moore, as these converters actually take some time. So it would be good to update the 'consumer' code which still uses v1 hypos (which is AFAIK both the ProtoParticle code and the persistency) to directly use the v2 hypos instead.... (and the persistency still uses v1 because the protoparticles still use it, and therefor that is what it still needs to support)
Edited by Gerhard RavenSorry @graven, I didn't realize that you are using aliases to refer to those converters
. Thanks a lot for all the clarification. Then, despite the LHCb::Event::Calo::v2::Hypotheses
container is general (can store photons,\pi^0, ...) these functions seem to create electrons and photons. Is the photon container meant to store any kind of neutral particle, or do we plan to split it depending on the particle ID?In the code the 'hypothesis' enum (eg. the 'thing' flagging whether an entry is photon/electron/merged pi0/photon from merged pi0/ pi0resolved/... ) is stored at the level of the individual elements of the container, so code-wise it is possible to 'mix and match' those within one container. However, IIRC, we currently just do not do that, and electrons, photons and merged pi0 happen to live in separate containers... but again, there is no technical reason for that, it's just (again, IIRC) how it's always used to be...
added 2 commits
18 18 class CaloHypothesesWithOrigin final { 19 19 20 20 protected: 21 std::vector<Gaudi::XYZPoint> m_origin; 21 std::vector<Gaudi::XYZPoint> m_direction; I was just playing around with the implementation, so it is simply a leftover from when I was working with the origin. It might be better to wait till I get to a fully working implementation (i.e. having
photons
and neutral pions working in HLT2) before starting reviewing this. I decided to create this MR in order to show the preliminary implementation of the zipping and the proposal for gathering values from function calls.
- Resolved by Miguel Ramos Pernas
- Resolved by Miguel Ramos Pernas
- Resolved by Miguel Ramos Pernas
added 62 commits
-
1bc25a51...d3ae34b0 - 60 commits from branch
master
- f2ef245d - Merge branch 'master' into mramospe-neutrals
- 37bcb1dc - Use 'private' instead of 'protected' for classes marked as 'final'
-
1bc25a51...d3ae34b0 - 60 commits from branch
added 1 commit
- 43c3cedc - Force the use of a comma after a call to PROXY_METHODS
added 4 commits
-
43c3cedc...6ce6e420 - 2 commits from branch
master
- ac5b2eb7 - Merge branch 'master' into mramospe-neutrals
- 85c4dce7 - Implement index-based accessor
-
43c3cedc...6ce6e420 - 2 commits from branch
added 1 commit
- ece4fb75 - Integrate the mass and PID accessors in the CALO objects container. Adapt it...
added 1 commit
- eb8c116e - Implement a 'copy_back' member function and allow to convert mask_v types to std::bitset