Implement passive material crossings in KinKal track extrapolation#1883
Implement passive material crossings in KinKal track extrapolation#1883RobMina wants to merge 5 commits into
Conversation
|
☀️ The build tests passed at ca33b1a.
N.B. These results were obtained from a build of this Pull Request at ca33b1a after being merged into the base branch at 1d2690a. For more information, please check the job page here. |
Issues found1. static const VEC3 nullfield(0.0,0.0,1.0e-3);Returning a fabricated 1e-3 axial field outside all maps avoids the 2. 3. Magic numbers / TODOs left as literals (low)
4. Stability caveat is a real blocker for enabling by default (medium) 5. Member-init reordering in |
|
📝 The HEAD of |
brownd1978
left a comment
There was a problem hiding this comment.
Hi Rob, this is great work, thanks. It will take me some time to go through the details. I have a few general comments/questions:
- Did you try using 'git mv' to rename the CRV extraplation function file? That generally keeps the commit history and line-by-line provenance
- The addition of ExtrapolateToR is logical. I wonder if that could be consolidated with ExtrapolateToZ by adding a closed cylinder surface with corresponding intersection function in KinKal.
- The Tracker perimeter is now being defined in several ways. Those should be consolidated, with values coming from GeometryService, along with a generalized toTrackerPerimeter function.
- Hard-coding a fixed field outside the range is fine for testing, but the underlying problem(s) needs to be addressed at its root.
…-unused max-R configuration for ExtrapolateToZ.
|
Hi Dave, Thanks for the review comments. The ExtrapolateCRV file changed >50% of the lines, so git registered it as a remove+add rather than a rename. I updated the tracker perimeter limits by reading the values from GeometryService and dropping the unnecessary fcl parameters. ExtrapolateToZ is still needed because it is used for the the tracker front plane by extrapolateTracker and for the TSDA by extrapolateTSDA (two different z planes that do not match the logic of the tracker perimeter). Reverted KKBField.cc -- the edits there were not needed for this PR. |
Add DS and concrete crossings during KinKal track extrapolation to the CRV (material was defined in the earlier PR #1868). Passive x'ings can optionally be stored in the KalSeed (off by default).
NB: to run stably, this still requires setting bfcorr=false (the current default) for CentralHelix fit/extrapolation. Running bfcorr=true CentralHelix requires upstream stability improvements in KinKal.