Skip to content

Fix minimize function in Measurement.cpp

Christoph Hasse requested to merge chasse_fix_meas_minimize1D into master

I originally had this in !2433 (merged) but think it's better off in its own MR.
I'd still like some more feedback, and make sure everyone agrees that this fix is the way to go before we merge this.
@ldufour @graven @wouter @smalde @sborghi

I originally only fixed the alignmentDerivatives in the TrackProjector.cpp but then looked more into the minimize() function and figured it's better to try and fix it globally.

The chronological explanation/history copied from !2433 (merged):

I found the bug looking at TrackProjector.cpp and the way it calculates the alignment derivatives... or rather in how it interprets the variable sMeas.
AFAIU, the intent of meas.trajectory().position(result.sMeas) is to calculate the POCA. If one looks at how this value is calculated in minimize(), the more sensible thing is to include a minus sign here.
I additionally checked this numerically and position(sMeas) is far away from the reference position of the track, while position(-sMeas) is quite close.
Proof:
FitNode reference state vector: x,y,z : -2959.68, 423.007, 9333.13
position(sMeas): (-3097.93,1994.9,9340.28)
position(-sMeas): (-2960.41,423.075,9334.62)

So instead of just fixing the above I now tried to fix the minimize() function:

I've pretty much convinced myself and believe the now committed version to be correct.

This does mean that the return values of what is called zState and sMeas were previously wrong.
Furthermore, this lead to the projection matrix having the wrong sign in its 3rd and 4th element.
Previously wrong: H: [ 0.904847 -0.00153319 -1.40311 0.00237747 0 ] ---> now correct: H: [ 0.904847 -0.00153319 1.40311 -0.00237747 0 ]
The values of H now also match what is produced by the independently derived formalism that is employed in the PrKalmanFilter.
@jadevrie I hope this might solve one of the sign problems we have observed! Only N-1 left to go 😂

I don't think there are configurations where we run this function with a StateZTraj that has non-zero B-field vector, but those cases were probably even more broken...

Some cleanup in this MR:

I was going to also fix the alignmentDerivatives in the MeasurementProviderProjector but instead removed them.
They aren't called anywhere, and the way the interface is defined right now you can't even call that function via a toolhandle anyway...

Edited by Christoph Hasse

Merge request reports