You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
What kind of change does this PR introduce? (Bug fix, feature, ...)
Several improvments to polyseeding workflow:
Taubin circle fit (Tpc_FittingTools.cc). The poly track fit used a simple algebraic circle fit. On short arcs it gives a too small radius, so the seed phi gets a bias and pT is underestimated. Now we use TrackFitUtils::circle_fit_by_taubin, the same fit as the CAseeding.
Seed circle center (TpcPolyTrackSeedConverter.cc). The circle center X0/Y0 of the converted TrackSeed was mirrored across the track (wrong sign). Now we take the center from the poly fit. Could affect only some specific QA.
Crossing period (4 headers). The poly modules used 106.56 ns per crossing, while Acts uses sphenix_constants::time_between_crossings = 106.652 ns. The converter adds the crossing back with one value and the fitter removes it with the other, so cluster z was off by about 7 um per crossing (2-3 mm for large crossings). Now all poly modules use the sphenix constant.
Polyseed Specific Residuals (Tpc_PolyClusterResiduals.cc). The tracks are fitted in the world frame, but the residuals used cluster positions in the TPC envelope frame. This gave a ~2 mm offset (two bands in r-phi). Now both are in the world frame. PS: it is not full track residuals module change, but polytracks only module.
Optional PHGarfield maps (PHGarfield.cc). If the space-charge or frame-charge scale is set to 0 on both sides (non-distorted MC flag), these maps are never used. A missing CDB payload is now not an error for them, so non-distorted MC also runs with tags that do not have the maps (e.g. MDC2).
Test: run 79513, 200 jobs x 1000 events, new build vs new + this branch, same macro, check resudialtree.
Matched tracks: pT ratio branch/release goes from 0.99 at low pT to 1.01-1.02 above 1.5 GeV. This is expected: the old fit gave too low pT, so tracks move to higher pT. Will partially solve mom-scale issue. The other part is eloss and it still will be there.
TPC-only tracks: the effect is much bigger, because pT comes only from the TPC fit. The ratio goes from 0.97 at low pT to about 1.1 at 1 GeV and like 2 at 3 GeV. Was not an issue in TPC only tests as those use refit before KF. But it was a porblem for TPC only analysis on final produced tracks.
TODOs (if applicable)
None
Links to other PRs in macros and calibration repositories (if applicable)
None
Motivation
The PR addresses polyseeding fit and coordinate issues that can bias reconstructed seed parameters and residuals. It also aligns crossing-time handling with the shared sPHENIX constant and allows unused Garfield map payloads to be absent when their scales are zero.
Key changes
Replace the local algebraic circle fit in Tpc_FittingTools::fit with TrackFitUtils::circle_fit_by_taubin.
Set converted TrackSeed X0/Y0 from the poly track’s helix center.
Compare cluster centroids and tracks in the world frame in Tpc_PolyClusterResiduals.cc when geometry is available.
Use sphenix_constants::time_between_crossings in the polyseeding modules instead of 106.56 ns.
Permit missing space-charge or frame-charge map payloads when both corresponding scales are explicitly zero.
The author reports improved pT ratios in a run 79513 comparison. Those results are author-reported and are not independently verified here.
Potential risk areas
The fit and seed-center changes affect reconstruction behavior, including inferred radii and track parameters.
The world-frame conversion changes residual and arc-direction calculations. Its fallback retains envelope-frame coordinates when geometry is unavailable.
The crossing-period update changes timing-derived positions; the commit notes an approximately 7 μm z shift per crossing.
The optional-map logic changes input-loading behavior when the relevant scales are zero.
No test results or current review findings were supplied. IO format, thread-safety, and performance effects are not established by the available evidence.
Possible future improvements
Add regression tests for short-arc fits, seed centers, residual frame conversion, and missing maps with zero scales.
Validate the reconstruction changes with the reported comparison and document the sample selection and uncertainty.
Track the remaining momentum-scale effects, including energy loss, separately from the circle-fit change.
AI-generated summaries can contain mistakes. Use best judgment and verify implementation details and reported performance against source and analysis results.
…tional field maps
- Tpc_FittingTools: use the Taubin circle fit; the algebraic fit underestimated R on short arcs, possibly giving a charge-dependent phi0 bias of the seeds.
- TpcPolyTrackSeedConverter: X0/Y0 were mirrored across the track; use the circle centre from the poly fit.
- Tpc_PolyClusterResiduals: compare clusters and tracks in the world frame (was a ~2 mm TPC envelope/world offset).
- PHGarfield: missing field-map payloads are not fatal when the correction is switched off (non-distorted MC, e.g. MDC2 tag).
The changes update Garfield map-loading conditions, transform TPC cluster centroids for diagnostics when geometry is available, and adjust TPC tracking defaults and calculations.
Missing space-charge maps are tolerated when both sides’ space-charge scales are explicitly overridden to zero. Missing frame maps are tolerated when both frame-charge scales are explicitly overridden to zero. Other unresolved inputs still cause loading to fail.
Cluster Residual Coordinates
Layer / File(s)
Summary
World-frame centroid use offline/packages/TrackingDiagnostics/Tpc_PolyClusterResiduals.cc
When ActsGeometry is available, cluster centroids are transformed from the TPC envelope frame to the world frame. Residual calculations, arc-direction selection, and recorded cluster coordinates use the transformed values. If geometry is unavailable, the code warns on the first event and uses untransformed coordinates.
These classes initialize m_crossingPeriodNs from sphenix_constants::time_between_crossings instead of the literal 106.56.
Taubin transverse circle fit offline/packages/tpctrackreco/Tpc_FittingTools.cc
Tpc_FittingTools::fit replaces its local Kasa fit with TrackFitUtils::circle_fit_by_taubin. It rejects non-finite radius or center values; subsequent checks and track-parameter calculations remain in place.
Helix center in published seeds offline/packages/tpctrackreco/TpcPolyTrackSeedConverter.cc
publishSeed uses the poly track’s helix-center coordinates instead of deriving the center from seed coordinates, curvature sign, radius, and azimuth.
Priority: High
Merge Risk: Moderate · up to 17f2f
The Taubin fit change depends on libtrack, but libtpctrackreco does not link it directly. A macro that does not load libtrack first could fail to load the polytrack library, so add -ltrack before merging. The residual diagnostics can still produce mismatched-frame residuals when no geometry node is present, which affects only the diagnostics output.
- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
process_event accepts saved clusters and tracks without ActsGeometry. The fallback then uses envelope-frame clusters. Tpc_PolyTrackReco requires ActsGeometry and stores tracks fitted from world-frame coordinates. A saved world-frame track container can therefore produce incorrect residuals in this no-geometry path.
The warning indicates an intended envelope-frame mode, but the code does not enforce that the tracks are also in the envelope frame. Require geometry for world-frame tracks, or define and enforce an envelope-frame track input contract. The previous implementation already used raw envelope-frame centroids, so this is a remaining diagnostics limitation rather than a new production-data defect. Major severity is not justified for this diagnostics-only impact.
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
TrackFitUtils::circle_fit_by_taubin(xy) is implemented in libtrack. Add -ltrack so libtpctrackreco does not depend on another module loading libtrack first.
I restarted jenkins on this, so that we can compare the current reference default build with new material map to this PR+default build+new material map
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Taubin circle fit, seed circle center, common crossing period, residual frame, optional PHGarfield maps
Types of changes
What kind of change does this PR introduce? (Bug fix, feature, ...)
Several improvments to polyseeding workflow:
Tpc_FittingTools.cc). The poly track fit used a simple algebraic circle fit. On short arcs it gives a too small radius, so the seed phi gets a bias and pT is underestimated. Now we useTrackFitUtils::circle_fit_by_taubin, the same fit as the CAseeding.TpcPolyTrackSeedConverter.cc). The circle center X0/Y0 of the converted TrackSeed was mirrored across the track (wrong sign). Now we take the center from the poly fit. Could affect only some specific QA.sphenix_constants::time_between_crossings= 106.652 ns. The converter adds the crossing back with one value and the fitter removes it with the other, so cluster z was off by about 7 um per crossing (2-3 mm for large crossings). Now all poly modules use the sphenix constant.Tpc_PolyClusterResiduals.cc). The tracks are fitted in the world frame, but the residuals used cluster positions in the TPC envelope frame. This gave a ~2 mm offset (two bands in r-phi). Now both are in the world frame. PS: it is not full track residuals module change, but polytracks only module.PHGarfield.cc). If the space-charge or frame-charge scale is set to 0 on both sides (non-distorted MC flag), these maps are never used. A missing CDB payload is now not an error for them, so non-distorted MC also runs with tags that do not have the maps (e.g. MDC2).Test: run 79513, 200 jobs x 1000 events,
newbuild vsnew+ this branch, same macro, check resudialtree.TODOs (if applicable)
None
Links to other PRs in macros and calibration repositories (if applicable)
None
Motivation
The PR addresses polyseeding fit and coordinate issues that can bias reconstructed seed parameters and residuals. It also aligns crossing-time handling with the shared sPHENIX constant and allows unused Garfield map payloads to be absent when their scales are zero.
Key changes
Tpc_FittingTools::fitwithTrackFitUtils::circle_fit_by_taubin.TrackSeedX0/Y0 from the poly track’s helix center.Tpc_PolyClusterResiduals.ccwhen geometry is available.sphenix_constants::time_between_crossingsin the polyseeding modules instead of106.56ns.Potential risk areas
Possible future improvements
AI-generated summaries can contain mistakes. Use best judgment and verify implementation details and reported performance against source and analysis results.