FEATURE: ACTS extrapolation to TPOT#4188
Conversation
…e track parameters to any TPOT surface for which a cluster was found at the seeding stage. This is similar to the extrapolation performed to TPC layers in distortion-targeted Silicon-MM fit. This will allow to obtain unbiased residuals in the Micromegas.
📝 WalkthroughWalkthroughAdds Micromegas-surface propagation during Svtx track post-fit updates when Micromegas is excluded from the fit, and updates dE/dx helper signatures to use a const thickness_per_region array and explicit pointer typing in TrackAnalysisUtils. Changes
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
offline/packages/trackbase_historic/TrackAnalysisUtils.cc (1)
168-178:⚠️ Potential issue | 🟠 MajorMissing
dedxlist.empty()guard —dedxlist.at(0)will throwstd::out_of_rangeon tracks with no TPC clusters.
calc_dedxcorrectly returnsquiet_NaN()whendedxlistis empty (lines 79–82).calc_dedx_calibhas no such guard: whendedxlistis empty,trunc_max = 0, the loop executes once (j=0 ≤ 0), anddedxlist.at(0)throws. With the current code path producingclusterKeysexclusively from the TPC seed, tracks carrying zero TPC clusters (e.g., a null or empty seed) hit this directly.🛡️ Proposed fix — mirror the guard already present in `calc_dedx`
+ if (dedxlist.empty()) + { + return std::numeric_limits<float>::quiet_NaN(); + } int trunc_max = (int) dedxlist.size() * 0.7; float sumdedx = 0; int ndedx = 0;
| auto* tpcseed = track->get_tpc_seed(); | ||
| float alpha = (r * r) / (2 * r * std::abs(1.0 / tpcseed->get_qOverR())); | ||
| float beta = std::atan(tpcseed->get_slope()); |
There was a problem hiding this comment.
tpcseed null-dereference latent risk — move outside loop and guard explicitly.
track->get_tpc_seed() is called once at line 100 (to build clusterKeys) and again at line 140 on every loop iteration. The only protection against dereferencing a null pointer at line 141 (tpcseed->get_qOverR()) is the implicit guarantee that if get_tpc_seed() returns nullptr, get_cluster_keys returns an empty vector and the loop is never entered. This coupling is fragile: if clusterKeys is ever populated through a different path (or if the call to get_cluster_keys at line 100 is refactored), a silent null-dereference becomes possible. Retrieve and guard the seed once, before the loop.
🛡️ Proposed fix
+ auto* tpcseed = track->get_tpc_seed();
+ if (!tpcseed) { return std::numeric_limits<float>::quiet_NaN(); }
+
std::vector<float> dedxlist;
for (unsigned long cluster_key : clusterKeys)
{
...
- auto* tpcseed = track->get_tpc_seed();
float alpha = (r * r) / (2 * r * std::abs(1.0 / tpcseed->get_qOverR()));
Build & test reportReport for commit ecddd62c08e23cb45a76caf175a5a72ffdeb7a84:
Automatically generated by sPHENIX Jenkins continuous integration |
Build & test reportReport for commit a139a428cc24a66a70ec0cd6af78ed4adebc6c04:
Automatically generated by sPHENIX Jenkins continuous integration |
osbornjd
left a comment
osbornjd
left a comment
There was a problem hiding this comment.
This will add a track state to the track object that in some senses is "misleading" in that it was not actually used in the fit. I don't have an objection to merging this (and it is possible I missed discussion related to this) but before merging this we need to make sure everyone is on the same page that the TPOT states, after merging this PR, mean something inherently different than all the other states in the track object
|
@osbornjd we can discuss this at tomorrow's tracking meeting. Still, using track state for storing extrapolation is already what we do for calorimeters, as far as I know. (and vertex ?). As well as for the TPC states in calibration mode. So in that sense it is not a new behavior. |
Right, my only thought is that the calos are known to be "special" and the calibration mode is "special." Again, I don't see a problem with it - it just means everyone needs to know that the TPOT state(s) will also be special |
|
Alternatively, perhaps we construct an |
I agree that this would be the cleanest, but also fear it is somewhat overkill ? My understanding is that state vectors are already an expert tool (aside from how many there are, nobody actually look at their value, except for making biased or unbiased residuals). In addition: right now TPOT is removed from the seeding entirely (so you won't get neither the cluster nor the state vector), in the official macro. For this feature to manifest, you need to
|
osbornjd
merged commit 4790cc5
into
sPHENIX-Collaboration:master
2 participants
When disabling Micromegas clusters from fit, use acts to propagate the track parameters to any TPOT surface for which a cluster was found at the seeding stage.
This is similar to the extrapolation performed to TPC layers in distortion-targeted Silicon-MM fit. This will allow to obtain unbiased residuals in the Micromegas.
Types of changes
What kind of change does this PR introduce? (Bug fix, feature, ...)
TODOs (if applicable)
Links to other PRs in macros and calibration repositories (if applicable)
ACTS Extrapolation to TPOT (Micromegas)
Motivation & Context
When Micromegas clusters are excluded from the track fit (e.g., distortion-targeted Silicon–MM fits), fitted track parameters are not available at Micromegas surfaces, preventing unbiased residual calculations. This PR adds ACTS-based propagation to extrapolate fitted parameters to TPOT (Micromegas) surfaces that had clusters at seeding, mirroring existing TPC extrapolation behavior and enabling unbiased Micromegas residuals when those clusters are not included in the fit.
Key Changes
Potential Risk Areas
Possible Future Improvements
Note on AI assistance: This summary was prepared with AI assistance. AI can make mistakes—please review the code diffs and run the usual validation tests to confirm behavior and correctness.