iLCSoft / MarlinReco

GNU General Public License v3.0
4 stars 40 forks source link

Update of the Track length calculator #112

Closed dudarboh closed 1 year ago

dudarboh commented 1 year ago

BEGINRELEASENOTES

ENDRELEASENOTES

tmadlener commented 1 year ago

I just had a brief look (so no detailed comments yet). There is a lot of removed code / functionality. I suppose this was mainly useful for development and testing and now that things have settled we really don't want to offer that functionality any longer since the helix length formula will be the only one that is used in the future? Otherwise, we could consider keeping the "legacy" functionality around in case one wants to redo these studies in the future again.

dudarboh commented 1 year ago

Indeed, I think I could keep few utility functions, without using them in the code itself, so that if we want to test all different options again, one at least has all the utils here, rather than deducing them mathematically again from scratch... I will add the onse I removed and one extra next week.

EDIT: As another point of improvement which is on the surface right now, that we could use reconstructed Primary and Secondary Vertex collections to get a starting point for the track length estimator, rather than always assume track comes from (0,0,0). It should be rather simple addition, but requires few more optional steering parameters for the vertex collections. I will try to implement this change locally and if I see it brings the improvement, then I don't see hard to include it in the next patch as well.

dudarboh commented 1 year ago

As discussed on the meeting today, we postpone the idea of calculating track length from the vertex position, as: a) for primary vertices, this should not make any difference, as (0,0,0) is just a reference point and distance to the PrimVertex accounted in z0 paramter. b) for secondary vertices, we keep it as it is, as for the current time-of-flight studies we always use time of the event, as the initial time, not the time of the secondary vertex production. So this needs a more involved treatment, even if needed.

dudarboh commented 1 year ago

As for the track state at the last hit discussion and the difference between getTrackState() vs propagate(). I would prefer to use getTrackState() for now for all the hits. As it seems to show reasonable results as long as the track length calculation is concerned and code looks cleaner. We can always implement this (probably) sub-percent level of improvement in the future.

So for me it looks good to go.