refactor: make whiteboard alias for tracks configurable #3917
Conversation
WalkthroughEnhancements to the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (4)Examples/Python/python/acts/examples/reconstruction.py (4)
The addition of Also applies to: 1323-1323
The Also applies to: 1525-1525
The Also applies to: 1600-1600 Line range hint Though changes look good, verify we must that existing code consuming tracks from whiteboard handles the aliasing properly. Run this script to find track consumers: ✅ Verification successfulHmm, safe the aliasing is, young padawan. Through the Force, I sense that all track consumers properly handle the whiteboard aliasing. The search reveals that:
Maintain this pattern, we do, where consumers read from the generic "tracks" alias while algorithms handle the proper aliasing of their specific output tracks. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Find code that reads from the whiteboard's "tracks" key
# This helps verify that the new aliasing won't break existing consumers
# Search for whiteboard track access patterns
rg -A 2 'inputTracks.*=.*"tracks"'
# Search for direct track references
rg '"tracks"'
Length of output: 11805 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
benjaminhuth
changed the title
add<Fitter>Tracks()
| @@ -1283,6 +1283,7 @@ def addKalmanTracks( | |||
| clusters: str = None, | |||
| calibrator: acts.examples.MeasurementCalibrator = acts.examples.makePassThroughCalibrator(), | |||
| logLevel: Optional[acts.logging.Level] = None, | |||
| whiteboardAliasTracks: str = "tracks", | |||
There was a problem hiding this comment.
hm I don't see why this is necessary for #3889 - the alias is something that should not need dynamic names. It is the thing that replaces dynamic names.
Looking at #3889 I think you can just do KF first and have "tracks" point to KF tracks and then do GX2F and have "tracks" point to this. Or you explicitly point to the "kf_tracks"
There was a problem hiding this comment.
Ah right, I didn't had in mind that we can reassign alieses, so it should be possible to first run+write the first fitter, and then run+write the second fitter.
Can you try that @AJPfleger? If this works, this change is not needed I think.
There was a problem hiding this comment.
yes, I am waiting for the physmon results. it worked locally :)
There was a problem hiding this comment.
@andiwand thanks for the suggestion. I didn't think, we could skip the aliasing. This is now a lot cleaner :)
This is helps to administrate multiple tracks, when using multiple fitters on the whiteboard.
Summary by CodeRabbit
whiteboardAliasTracksparameter across multiple functions.