Fix to the cluster key init#4171
Merged
osbornjd merged 1 commit intosPHENIX-Collaboration:masterfrom Feb 9, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe private member 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 |
Build & test reportReport for commit f310306304826443ee4e2d8217fbd4c1efcd70f1:
Automatically generated by sPHENIX Jenkins continuous integration |
Contributor
|
Just to check, @pinkenburg this does not cause any problem with DST readback since there are no new objects (right)? It just means in old DSTs the old value will still be used in the initializer |
Contributor
|
yes - this won't change the readback. root does call the ctor when reading the data back, so the initial value will change from 0 (which is what {} does) to uint64_t (max). But then the dst content will overwrite this |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Types of changes
What kind of change does this PR introduce? (Bug fix, feature, ...)
Fix the initialization of the cluster key. Previously it was being instantiated with {} (that would be 0), which made the state at the vertex be wrongly identified as a MVTX state. Now it is instantiated as std::numeric_limits<uint64_t>::max()
Fix to Cluster Key Initialization (PR #4171)
Motivation / Context
The cluster key (
_ckey) member inSvtxTrackState_v3was initialized with an empty initializer{}, which evaluates to 0. This caused a critical misidentification issue: uninitialized track states were incorrectly being flagged as associated with MVTX (Miniature Vertex Tracker) detector clusters, since 0 is a valid cluster key value. This fix introduces a sentinel value to distinguish genuinely uninitialized cluster keys from valid detector state assignments.Key Changes
SvtxTrackState_v3classTrkrDefs::cluskey _ckey{};TrkrDefs::cluskey _ckey{std::numeric_limits<uint64_t>::max()};offline/packages/trackbase_historic/SvtxTrackState_v3.hSvtxTrackState_v2.hretains the old initialization pattern for backward compatibilityPotential Risk Areas
get_cluskey() == 0or similar checks to identify uninitialized states must be updated to use the new sentinel valuePossible Future Improvements
is_valid_ckey(),is_initialized_state()) to encapsulate these checksNote: This summary includes AI-assisted analysis. Please verify the impact on downstream reconstruction algorithms and existing data files, particularly for physics analyses that may depend on vertex-state identification logic.