bug: KF verbosity statement

[osbornjd]

This is not a critical bug, but there is a verbose statement in the Kalman Fitter that causes a seg fault when running with verbose output. This statement here can cause a crash when the KF is propagating in the final stage back to the perigee surface when the perigee is created like it is in the track fitting algorithm here since it does not have an assigned geometry ID.

I create an issue as there are two solutions, and I don't know which the development team suggests (I'm happy to implement either)

1. Change the verbose statement to not assume a surface has a geometry ID. This will make the verbose statement potentially less useful without the ID.
2. Assign a geometry ID to the perigee surface at construction time (is this even possible?).

[andiwand]

Thanks for discovering and reporting this @osbornjd !

From the implementation it looks like the GeometryIdentifier should be default constructed if none is passed down to the GeometryObject. Do you know where the segfault happens?

One solution could be to check if the ID is present else don't print it.

In general the extrapolation to the reference surface in the core KF should go away anyway IMO. This would also fix the problem. But I fear it might take a bit longer to do that.

[osbornjd]

Do you know where the segfault happens?

GDB says it happens when calling the geometry identifier in this line. I checked this myself also by adding in print statements as I also thought the geometry identifier should be default constructed - but apparently it is not when doing something like what is in the track fitting algorithm I linked originally. I should also mention that we are running v30.3.0, so perhaps this has been updated in a more recent version. I'm happy to try some other tests if it would be useful for pinning down what actually is the root cause of this segfault.

In general the extrapolation to the reference surface in the core KF should go away anyway IMO.

In my experience experiments do not want this to go away as they really want the track parameters WRT some line surface, e.g. (0,0,0) or the production vertex or whatever. That is how the KF is used in sPHENIX and, although the CKF is used in ePIC, the final fitted track parameters are required WRT the line surface for physics analysis.

[andiwand]

Would be great if you could check it against the current version. Potentially you can just update this file so you don't have to deal with breaking changes.

In my experience experiments do not want this to go away as they really want the track parameters WRT some line surface, e.g. (0,0,0) or the production vertex or whatever. That is how the KF is used in sPHENIX and, although the CKF is used in ePIC, the final fitted track parameters are required WRT the line surface for physics analysis.

Right this is where https://github.com/acts-project/acts/blob/22730e1e5faa4abb218289269448d89e412b1430/Core/include/Acts/Utilities/TrackHelpers.hpp would come in. I think the core algorithms should be as compact and flexible as possible. Adding the extrapolation in seems to me rather like a component/helper of Acts and not a feature of the algorithm. Other fitters would have to do the same potentially duplicating code.

I would either propose to remove it and let the user have control over the extrapolation or separate this strictly in the algorithm by having a fitter function without extrapolation and one with for the convenience of the user.

This is what I did in https://github.com/acts-project/acts/pull/2722 with the CKF and I think we should do this with the fitters do.

But this is only my opinion. I am happy to discuss this and find some agreement before we start working on this.

[osbornjd]

Would be great if you could check it against the current version. Potentially you can just update this file so you don't have to deal with breaking changes.

I'll just update to the new version and run the example with verbose mode on. In principle it should be the same since the perigee surface is created the same way in TrackFittingAlgorithm

[github-actions[bot]]

This issue/PR has been automatically marked as stale because it has not had recent activity. The stale label will be removed if any interaction occurs.

[AJPfleger]

Hi @osbornjd, could you reproduce the crash with the newer version or should we close this issue?

[github-actions[bot]]

This issue/PR has been automatically marked as stale because it has not had recent activity. The stale label will be removed if any interaction occurs.