-
Notifications
You must be signed in to change notification settings - Fork 15
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
GlbCurvilinear <--> CCS conversions #324
Conversation
Note: to confirm that the symbolic/analytical math is correct, the new conversion was debugged from inside CMSSW The CCS to curvilinear transform was checked in that same setup comparing to the original curvilinear covariance. The code was then moved to TrackState class with a switch to float. |
@cerati others are welcome. |
@slava77 I'm no expert here, but I'll ask anyway... if I interpret correctly, blue = CKF, red = mkFit/before, black = mkFit/after, and the plots come from http://uaf-10.t2.ucsd.edu/~slava77/figs/mic/mtv/10muHS_mk48b9f5b_sw5766e9f_vs_25bd4b9/plots_ByOriginalAlgo_initialStep/effandfakePtEtaPhi.pdf. If so, these results look great, especially in the endcaps. Given that the remaining accuracy issues are associated with pT > 100 GeV, how likely do you think it is that the remaining efficiency gap in the barrel would basically go away if mkFit coord transforms and covariance calculations were done in double precision? (It would undoubtedly have a bad effect on timing performance, of course - but could it be tried?) |
yes, sorry for missing to note this. I've updated the PR description |
I looked at a few cases with bad covariance coming from mkFit and the cases were in the more forward region. Because of this I suspect that the issue is at least not the end points of converting to the curvilinear covariance, and chances are that there are other reasons for inefficiency. I think that it is more likely that some issues arise from the remaining KF machinery in single precision, but I wouldn't bet that this has to be the case. I suggest to investigate the failed tracking cases in the high-pt end after this PR is merged and then see if the issue can not be mitigated without going to double precision. |
comment post-merge: looks good to me! |
new conversions are added in
TrackState
to convert directly to/from the CMSSW parameterization using the curvilinear covariance matrix.The reference CMSSW state is assumed to be what would be obtained from the FreeTrajectoryState: the state is in global 6D (p3, r3) reference frame and the 5x5 covariance in curvilinear frame filled in the upper part of the
TrackState
6x6 error matrix.This parameterization is preferred since it has a much smaller numerical uncertainty introduced from the CMSSW to mkFit/CCS covariance. A conversion from the Cartesian covariance includes a rotation together with coordinate transfrorm from q/p to {px,py,pz}, which degrades numerically beyond the single precision around 100 GeV. In mkFit the covariance matrix is in single precision.
tested with a matching update in CMSSW on 10mu (highpt muons) data
http://uaf-10.t2.ucsd.edu/~slava77/figs/mic/mtv/10muHS_mk48b9f5b_sw5766e9f_vs_25bd4b9/
blue = CKF, red = mkFit/before, black = mkFit/after, initialStep built tracks show a significant recovery in efficiency:
the total number of non-positive-definite covariances coming from mkfit (a sum over all tracking iterations) goes down from 17327 to 792 on 10000 events. The remaining cases appear to be originating in mkFit, while the original was dominated by the Cartesian -> CCS conversion.