Don't use radiusOfInnermostHit for EDM4hep tracks

[jmcarcell]

and compute it from hits when going from EDM4hep to LCIO. See https://github.com/key4hep/EDM4hep/pull/326. Uses the utility to compare floats introduced in https://github.com/key4hep/k4EDM4hep2LcioConv/pull/71.

Currently there is an issue in the comparisons because in some LCIO collections the radius of the inner most hit doesn't match what you get from the list of hist associated with the track, which isn't enforced anywhere that it should be. One option is to loosen the comparison criteria.

BEGINRELEASENOTES

• Don't use radiusOfInnermostHit for EDM4hep tracks, compute it from track state at first hit when going from LCIO to EDM4hep.

ENDRELEASENOTES

[tmadlener]

Looking a bit through the iLCSoft reconstruction chain, these are the places where the radius is set non-trivially (i.e. not simply copied from another track). In most of the cases it is set from the referencePoint that is obtained from a TrackState, which according to the documentation most likely is the position of the first hit (if the TrackState is filled appropritately). In the following cases the 3D(!) position is used for this.

• https://github.com/iLCSoft/MarlinTrkProcessors/blob/5503ee6a012e56e71e18c46f9cbe765e626b7d24/source/Refitting/src/DDCellsAutomatonMV.cc#L1348C20-L1349
• https://github.com/iLCSoft/ForwardTracking/blob/bd6ef3f58dc601266a835d1344682cbf94710de3/src/ForwardTracking/ForwardTracking.cc#L1327-L1328
• https://github.com/iLCSoft/ForwardTracking/blob/bd6ef3f58dc601266a835d1344682cbf94710de3/src/ForwardTracking/SiliconEndcapTracking.cc#L1470-L1471

Then there are two cases where the same information is used, but only the x-y position is used for calculating the radius:

• https://github.com/iLCSoft/Clupatra/blob/f9847b5d170bee4bc29e0c406c20c22f0de0e9c7/src/clupatra_new.cc#L1281-L1284
• https://github.com/iLCSoft/Clupatra/blob/f9847b5d170bee4bc29e0c406c20c22f0de0e9c7/src/ClupatraProcessor.cc#L1353-L1358

Finally, there is one occurence, where the position of a TrackerHit is used directly, also in x-y only.

• https://github.com/iLCSoft/MarlinTrk/blob/5941fde4b5a24461d60d0bc40c0f75356cbcfad9/src/MarlinTrkUtils.cc#L703-L705

If we want to have the same behavior as for the first three cases here, we would have to shift the filling of this until after we have all the track states converted and then simply use the same approach as there and then document how we fill this information during the conversion from EDM4hep to LCIO.

[tmadlener]

I have introduced a utility function that gets the radius from the track states in a track. It defaults to only using the x and y coordinates, but has a toggle to go to 3D. Currently the 2D version is used in the conversion of the tracks. In order to make all the existing tests pass, we compare against both and are happy if one of them passes. I think the latter is OK because the roundtrip tests pass otherwise as well, and for a direct comparison between converted EDM4hep files and the original LCIO file the "correct choice" depends on which track collection you are looking at and what you actually want to do.

From my side this is ready for review and merge

[jmcarcell]

I made a couple of comments but this is one of the problems that doesn't have an answer after removing the equivalent quantity. So the possible affected use case would be to go from EDM4hep to LCIO and then using the value of the radius in some processor which I guess no one has (but what if we have tracking in EDM4hep at some point and then keep using the rest of the LCIO chain?). Maybe @Zehvogel can comment on which way he would like the radius to be set. Other than that my doubt is whether to keep the bool parameter or just commit to a single option (even if wrong for some collections).

[Zehvogel]

In the ILD files that we have for testing it looks like the choice is not even consistent within a single collection.

Yes it is quite the mess, but this bad? :o

I think the most consistent one should be the 2D as that is the one set in finaliseLCIOTrack https://github.com/iLCSoft/MarlinTrk/blob/5941fde4b5a24461d60d0bc40c0f75356cbcfad9/src/MarlinTrkUtils.cc#L703-L705 Which I thought would also be called at least for all "end-user" ILD tracks? :) It seems to be also what Clupatra does. The forward tracking however prefers the 3D one... Just adding the search link for completeness https://github.com/search?q=org%3AiLCSoft+setRadiusOfInnermostHit&type=code

TL; DR:

I agree, there is no right choice here or the conversion.

I agree :(

[tmadlener]

Alright, I will merge this then to unblock the EDM4hep PR.

[gaede]

For whatever it is worth: the use of a '3d' radius in ForwardTracking is a bug that obviously has gone unnoticed for many years.