Draft: Add End RICH2 state locations to Ttrack template

added RTA RTA WP2 labels

requested review from @cmarinbe and @decianm

assigned to @isanders

mentioned in issue Moore#418 (closed)

mentioned in merge request Moore!1477 (merged)

Do you need both, BeginRich2 and EndRich2?

added 69 commits

9f0e0089...6ea5c731 - 66 commits from branch master
cda1152e - Merge branch 'master' into isanders-ttracks-v3-fix
4e682e35 - Merge branch 'master' into isanders-ttracks-v3-fix
448f72e0 - removed SL::BegRich2 from T track template

Compare with previous version

resolved all threads

/ci-test

Started integration test build. Once done, check the results or the comparison to a reference build.

Throughput Test Moore_hlt1_pp_default: 28381.6 Events/s -- change of -0.11% vs. reference

Throughput Test Moore_hlt2_fastest_reco: 504.6 Events/s -- change of -0.01% vs. reference

Throughput Test Moore_hlt2_pp_thor: 295.5 Events/s -- change of -0.09% vs. reference

There are a number of "Invalid state" warnings. @isanders can you have a look ?

I've tried having a look at where this is coming from but can't make sense of it. I think it has its roots here: https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/Event/TrackEvent/include/Event/SOATrackConversion.h#L129 . @decianm do you have any ideas? Is it an issue that the v1 tracks don't also have an EndRich2 state?

Wait I think I found the cause here https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/Event/TrackEvent/include/Event/SOATrackConversion.h#L102 so unless I am mistaken it looks like it is looking for the state on the v1 track... not sure what needs to be done

Hey @decianm -- do you know what needs to be done here? I think it's complaining that it can't find the new v3 state location on the v1 track... do I also need to make changes to Track_v1 somewhere?

Hello @isanders

I haven't forgotten this, but I am a bit busy with other things at the moment. I'll try to come back to it by the end of the week.

I think your conclusion is correct, it obviously can only convert a state from v1 -> v3 if it is not already on the v1 track. However, I don't think it makes sense to add this state to all v1 T tracks, I understand you only need this for your special situation.

I did a bit of thinking, I think the cleanest solution would be to add a constructor to v3::Tracks that accepts a custom state_collection (or a state_location) that is then used (or added) instead of the pre-defined one for Ttracks. I am afraid this needs a few lines of coding, but at least it would also solve this for future instances of the problems with other track types (I hope...).

Many thanks for the reply. I have been having a look in the code over the past week on how to do this, but I am not sure where to start. Would you be able to point me to where the current constructers are implemented in the code? I'll see then if I can figure it out. Cheers

The 3 current constructors are here: https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/Event/TrackEvent/include/Event/Track_v3.h#L205

Thanks a lot! I am trying to wrap my head around the thing. Right now, looking at the code, I do not see how an argument in the constructor would change things, rather should we add another type of Ttrack, like:

  template <>
    struct available_states<TrackType::TtrackExtended> {
    using type = state_collection<SL::FirstMeasurement, SL::LastMeasurement, SL::EndRich2>;
  };
  struct available_states<TrackType::Ttrack> {
    using type = state_collection<SL::FirstMeasurement, SL::LastMeasurement>;
  };

?

You can in principle add a new type, but this means you need to add it to all the code that ever does an if for a Ttrack, which is not particularly nice. It is also not very flexible, because I am afraid this problem will come up more in the future and we would then need something like TrackType::TtrackMoreExtended or so . And lastly, it is conceptually still a T track.

I'll try to find some time for quickly checking what I intended in the next days (i.e. giving optional state locations during the construction phase), which would make it more flexible for all use cases in the future.

Thanks a lot! When I had a look yesterday, the other plan (following your lines) I could come up with was:

adding an optional list of states to the default constructor;
add those states to the track if they are there.

The issue I got is, firstly, I do not know the concise way to add states, and second, do we want to have the EndRich2 state in some V1 tracks and thus tell the converter to add them, or are they not in the V1 and so we need to extrapolate.

I am not sure I fully understand, but I thought (at the moment) this is purely a V1 -> V3 conversion problem (?). You can add as many states as you want to a v1 LHCb::Track, but the problem is that the converter then cannot distinguish if it has to convert an additional state or not, as it only takes the fixed list for Ttracks.

So yes, conceptually my idea also was:

Add a list of optional states to v3::Tracks.
Make sure the track "understands" them (i.e. takes them into account when calling has_state
Make the v1 -> v3 converter take these additional states into account as well