Default track fit initialisation based on hits is not reliable?

[dudarboh]

Description

The MarlinDDKalTestTrack::initialise( bool fitDirection ) which creates initial helix for fit based on 3 hits, doesn't work with 1D (strip) hits.

Which makes it unreliable option for the ILD which has SET (with strips), making fit initialization to fail quite often.
It may also fail for very boosted tracks which have their first hit in the FTD (with strips) omitting VTX.

So far I looked, there is no place where we use default initialization in the production code.
E.g. during the final refit, if one fails to find a track state AtLastHit one creates track state at the IP in order to use working initialise( trackState, bfield_z, fitDirection ) which relies on the input track state, but without mentioning anything about existing default hit-based initializer..

Spreading unawareness of this might results in multiple processors, providing fit option which doesn't work.
E.g.:
Currentl RefitProcessor with the default option for the track fit initialization, doesn't work throwing an exception about 1D hits mentioned above. While production uses option 3 - AtLastHit it raises no awareness on why so and it can easily be forgotten through time and lead to bugs.

My second thought was that the default initializer is meant to work with MarlinAidaTTTrack.
However, MarlinAidaTTTrack::initialise( bool fitDirection) relies on calculateStartHelix(), which as far as I understood removed since v02-02-01 with MarlinTPC?

Shall one think of:

1. is it possible to provide helix initialization if 1 of the hits is 1D? If so, then it shall be implemented.

2. Making a very BIG warning or improving thrown exception
   The current throw message is: "Track fit cannot be initialised from 1 Dimensional hits. Use method MarlinDDKalTestTrack::initialise( bool fitDirection )", which could be improved:
   a) it's misleading and probably missing "Don't use method...".
   b) User will probably call MarlinTrk::createFinalisedLCIOTrack(. . .); and will not call initialise() directly, so the message is not helpful for the user on how to fix it. Saying e.g. - "provide viable pre_fit track state" would help much better.
   c) It should explain to the user that it is not his fault, but just not working option in the software

3. Do we need it at all? Removing default initializer which doesn't work by design with current ILD setting and has no use?

[dudarboh]

Oh, I wasn't thoughtful enough with the code. I found some fixing behavior in
createPrefit( std::vector<EVENT::TrackerHit*>& hit_list, IMPL::TrackStateImpl* pre_fit, float bfield_z, bool fit_direction)

but it is only called by:
int createFinalisedLCIOTrack( ... , const EVENT::FloatVec& initial_cov_for_prefit, float bfield_z, double maxChi2Increment)
and not
int createFinalisedLCIOTrack( ... , EVENT::TrackState* pre_fit, float bfield_z, double maxChi2Increment)

Assuming that if user wants to use default pre_fit user will always call method with cov_matrix. But user might not know about that... It seems straightforward to add the same line to the second method

[dudarboh]

It was actually intentionally changed a while ago here:
iLCSoft/MarlinTrkProcessors@3a6683d?diff=unified

@gaede do you recall the reason for this change?