refactor: avoid addWhiteboardAlias in truth tracking examples
#3936
Conversation
WalkthroughChanges made in the pull request focus on the Changes
Possibly related PRs
Suggested labels
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
Examples/Scripts/Python/truth_tracking_kalman.py (1)
Line range hint
126-164: A pattern of wisdom, established this has!Across all three tracking implementations - GSF, GX2F, and Kalman - a consistent pattern now flows. Direct references to selected tracks, clarity they bring. Future maintainers, grateful they will be, hmm.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
Examples/Scripts/Python/truth_tracking_gsf.py(3 hunks)Examples/Scripts/Python/truth_tracking_gx2f.py(3 hunks)Examples/Scripts/Python/truth_tracking_kalman.py(3 hunks)
🔇 Additional comments (3)
Examples/Scripts/Python/truth_tracking_gsf.py (1)
126-126: Consistent use of selected tracks, approve I do!
Wise changes these are, hmm. Direct reference to "selected-tracks" now exists, matching the selector's output it does. Clarity this brings to the code's intent, yes.
Also applies to: 138-138, 149-149
Examples/Scripts/Python/truth_tracking_gx2f.py (1)
126-126: Harmonious changes across implementations, I sense!
Like its GSF companion, this file now speaks clearly of selected tracks. Consistency across the codebase, brought this has. Strong with the Force, these changes are!
Also applies to: 138-138, 149-149
Examples/Scripts/Python/truth_tracking_kalman.py (1)
142-142: Complete the circle, these changes do!
Like its siblings, this implementation now speaks of selected tracks directly. A path to clarity, this change creates. Removed the confusing aliases are, hmm.
Verify the removal of aliases and their impact, we must:
Also applies to: 154-154, 164-164
✅ Verification successful
Consistent and harmonious, these changes are!
In all three truth tracking implementations - Kalman, GSF, and GX2F - the same pattern of "selected-tracks" we see. No whiteboard aliases in these tracking files remain, hmm. A unified approach, this creates!
The Force shows us that other files still use addWhiteboardAlias, but for different purposes they do - simulation and reconstruction their domains are. Disturb them, these changes do not.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining addWhiteboardAlias usage in truth tracking examples
rg "addWhiteboardAlias" "Examples/Scripts/Python/truth_tracking_*.py"
# Verify consistent usage of selected-tracks
rg "selected-tracks" "Examples/Scripts/Python/truth_tracking_*.py"
Length of output: 304
Script:
#!/bin/bash
# First, find the correct path using fd
fd "truth_tracking_kalman.py"
# Search for any remaining addWhiteboardAlias usage in Python files
rg "addWhiteboardAlias" --type py
# Verify consistent usage of selected-tracks in Python files
rg "selected-tracks" --type py
Length of output: 4850
|
The main idea with reassigning |
|
|
@andiwand oha, that seems useful 😊 |
| @@ -73,7 +73,6 @@ def runTruthTrackingKalman( | |||
| outputParticles="particles_input", | |||
| ) | |||
| ) | |||
| s.addWhiteboardAlias("particles", "particles_input") | |||
There was a problem hiding this comment.
@andiwand Do you remember if that alias is actually needed? We have it only in the KF but do not even use it here. Potentially, we should also add it to the other chains then.
There was a problem hiding this comment.
I think we were using "particles" in some downstream truth matching or simulation at some point. As far as I can see this should be "particles_selected" these days and "particles" might not be used anymore.
I think this is one of the paths in the script we never test?
I think this was never officially discussed. I implemented that after facing several problems without aliases. My thought was that for examples |
The aliases might be confusing in these examples, since they are redefining previously used names. This might be a bit clearer, which objects are used and makes it hopefully easier to understand the examples.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation