WIP: Add TrackNProngVertex and TrackCompactVertex

This is a proposal for two new utilities:

• a new 'fast' N-prong vertex fit (called TrackNProngVertex)
• a class to store the result of the vertex fit such that p4 (and cov 7x7) can be computed after mass assignment to daughters). This would be the thing to store in the event store.

Both structures are templated on the floating point type and on the number of daughters: the latter can either be an integer with the actual number of daughters (in which case daughter information is stored in a std::array) or 'Dynamic' (standin for c++20 std::dynamic_extent, which didn't yet exists on my platform) in which case a vector is used.

There are still lot's of FIXME's in the code, but can be input for discussion in the next tracking meeting. (WH, 2019-04-01)

Great to see @wouter ! I am at a workshop so won't connect tomorrow but could you please open a complementary "vision" page at

https://gitlab.cern.ch/lhcb-rta/direction

so we can factorize the "design" and technical/code discussions?

One suggestion triggered by:

Both structures are templated on the floating point type and on the number of daughters: the latter can either be an integer with the actual number of daughters (in which case daughter information is stored in a std::array) or 'Dynamic' (standin for c++20 std::dynamic_extent, which didn't yet exists on my platform) in which case a vector is used.

and inspired by the text at the bottom of boost small_vector

small_vector<T, N, Allocator> is convertible to small_vector_base<T, Allocator>, a type that is independent from the preallocated element count, allowing client code that does not need to be templated on that N argument. small_vector inherits all vector's member functions so it supports all standard features like emplacement, stateful allocators, etc.

I think the same applies here -- it would be preferable to have a way of using the container without specifying the details of how it is allocated behind the scenes. In both case, the memory layout of the payload is the same, so one should be able to provide the equivalent of a span to abstract away the differences without any loss of performance.

added 436 commits

• a35d7fc4...ca44a28c - 435 commits from branch master
• e3992c36 - merged with master

Compare with previous version

added 1 commit

• df966e37 - add copyright

Compare with previous version

@graven Can you explain me what you had in mind (hiding static/dynamic storage for objects stores in event store)? Thanks ...

start with a base class which has all the functionality, except that it only has a 'view' of the memory -- i.e. all the member functions, but which only has a pointer to the memory of the payload, and some pointers to functions to de-allocate, move and assign the payload (*). Next, define two different types of derived classes (the distinction between them can be template specializations!) and the only thing these do is to make sure the memory exists, and call the base class with a pointer to the memory they allocated, and pointers to the functions to de-allocate, move and assign (and maybe swap) (basically, write your own little explicit v-table to which all the 'special members' (destructor,copy/move constructor, (move)assignment) can dispatch -- implemented maybe similar in technique to this snippet)

(*) Basically, see eg. the description for boost::small_vector here which states (amongst others)

small_vector<T, N, Allocator> is convertible to small_vector_base<T, Allocator> that is independent from the preallocated element capacity, so client code does not need to be templated on that N argument. All boost::container::vector member functions are inherited. See vector documentation for details.

You can also take a look at the boost source code and note that boost::small_vector is 'just a bunch of constructors' and all the guts are in boost::vector, which has a very interesting and non-trivial destructor, (copy/move)constructor and (move) assignment.

added 1 commit

• eff63042 - Fixed formatting

Compare with previous version

added 1 commit

• 5a00efc8 - undid change by format patch in vector constructor

Compare with previous version

added 1 commit

• d721fad4 - apply formatting patch

Compare with previous version

@graven am I right that you would need to play this game at two "levels", if you want to be able to store something like vector<TrackNProngVertex<N>> on the TES and then access it generically (i.e. in algorithms that are not templated on N)?

Reading your suggestion I guess you would both need to have TrackNProngVertex inherit from TrackNProngVertex_base (following the recipe of small_vector_base), but you would also need a special container type? i.e. you store VertexContainer<TrackNProngVertex<2>> on the TES and then subsequent algorithms take VertexContainer_base const&, whose interface returns TrackNProngVertex_base const&? Something very, very(, very, very) roughly like https://godbolt.org/z/gI4Pkt ..? Or am I missing some way that this can be done using standard containers?

Hi Olli, I didn't look at this issue anymore. I guess the only disadvantage is that you need one extra pointer (the one in the base class that points to the payload. Could Gerhard and you perhaps decide next week what to do? Thanks ...

I chatted to @graven about this at the hackathon. We agreed that it is simpler to return two containers from the vertex producer, one that manages the lifetime of the (N-dependent) data, and another that provides a generic interface that is used by other algorithms. I think that https://godbolt.org/z/s8mw2h is a rough sketch of what we were discussing.

When I was writing down the example I thought that https://godbolt.org/z/iheWyO is also a reasonable option, it is a little more awkward to use in the producer [this could maybe be improved with the SOA::* family], but might be a slightly better memory layout for accessing N-prong-independent quantities like the chi2. I am sure @graven will correct me if I have misrepresented what he was saying!

Indeed, your https://godbolt.org/z/s8mw2h is just what I had in mind -- separate ownership and usage, and use the fact that the TES is const, and can thus be used as 'backing store' to a non-owning interface -- and hence we can have templated data-owners that are statically exactly the right size, and a single 'dynamic-size' interface which acts as the front end to any such fixed-size owner.

added 1 commit

• d7028a12 - some renaming

Compare with previous version

added 1 commit

• bc13bc51 - some renaming

Compare with previous version

added 41 commits

• bc13bc51...fa5c6e10 - 40 commits from branch master
• 50f51178 - Merge branch 'master' of ssh://gitlab.cern.ch:7999/lhcb/Rec into WH_TrackNProngVertex_201903

Compare with previous version

mentioned in merge request !1534 (closed)

added 1 commit

• b4b039f0 - added copyright; applied formatting patch

Compare with previous version

added 1 commit

• acc861ad - Fixed formatting

Compare with previous version

mentioned in commit 73d47491

mentioned in commit 8215c493

mentioned in commit 7eaadb21

mentioned in merge request !1625 (merged)

mentioned in commit 9f846bbc

mentioned in commit bee7573f

I just noticed that this is still open, but [I think all of] its content was merged in !1625 (merged). Closing this one to tidy up.

closed

Can I also delete the branch?

I would say yes, but perhaps @wouter could confirm.

I think that all the changes are in master already except for the TrackPVChecker algorithm that is included in the still-open MR !1290 (closed).

mentioned in commit LHCb@3f1c0fdb

mentioned in commit LHCb@56c6adf4

mentioned in commit LHCb@77f13f12

mentioned in commit LHCb@5fddeadd

mentioned in commit LHCb@0a5b0425

mentioned in commit LHCb@993567f6

mentioned in commit LHCb@3c3211f0

mentioned in commit LHCb@66f08083

mentioned in commit LHCb@5e1c7dcc

mentioned in commit LHCb@5cc48ff1