-
Notifications
You must be signed in to change notification settings - Fork 12
Retrieve t0 errors from LC relations #1138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Originally noticed when looking into the track extension and checking hits and their errors:
|
Comparing the original and new fits from the old code, they are quite different even when the refit doesn't fail:
|
Hey Matt. One thing is that the global fit t0 may have been carried correctly. I never had a problem getting hit times to work and be carried over; I did get hit time errors having a problem and on my end I had some janky stuff I did (which I never pushed) to circumvent it. So you are right; the default would have bungled t0err and this is the proper fix for the errors. The issue I see with the times is that all the times are shifted by some default value (something close to 32 maybe) which I believe could very well be some global time shifting done in earlier steps that the lcio read in times does not do. In RawHitFitterDriver there are like 6 different timing corrections that are somewhat complicated (though typically come out to a net 30 ish ns) that you may skip by not using the original time. Nothing touches the time error in this, but I think it would be appropriate to use original time because I think that is propagated correctly. I would need to do some tests, but a simple test is plotting the track times and seeing what that looks like now |
Looks like you didn't change how it draws time, so that should be fine I believe. I was just dubious about that new time list you gave for t0. |
Wait something does modify the errors somewhere; I did some error changes to get the layer dependent chi squared distributions to look correct. That may bungle stuff here in ways similarly complicated to how t0 would be bungled if you used early versions of it. |
Looking at #1088, it seems that all the changes are done in RawFitHitterDriver and prior to clustering or later steps, and the fact you draw from "SVTFittedRawTrackerHits" (which I believe is the collection you get after this processor and not before) means it should be fine. Thats the only thing I think fiddles with time error stuff so I believe you can draw from it without concern that some fix wasn't applied (unlike the t0 thing). |
The t0Errs from the refit were clearly plagued with issues, and the logic as it stood would return in bogus time errors being propagated downstream. In various situations, the time error would be 1e8, infinity, etc...
In this PR, the fitted pulses are instead retrieved from an LC relations table. This makes consistent the errors with the t0 value. From quick tests, this seems to yield very reasonable numbers.
The previous code also ignored layer 0. It's unclear to me why that was done. The errors look fine to me from local tests.
Note, this will affect the track time as computed using the inverse time variance calculations:
#1089