ci: Reenable some ExaTrkX tests #3395
Conversation
|
don't let your dreams be dreams! |
Renable builds, skip tests until issues resolved in #3395.
|
|
WalkthroughReintroduced in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.gitlab-ci.yml(1 hunks)
🔇 Additional comments (1)
.gitlab-ci.yml (1)
145-157: Verify GPU presence before tests run, we must.
Previous wisdom shared, still valid it remains. Before ExaTrkX tests begin, GPU availability check we need. Prevent mysterious failures on non-GPU runners, this will.
script:
- source CI/dependencies.sh
+ - nvidia-smi || (echo "GPU not found. Required for ExaTrkX tests." && exit 1)
+ - python3 -c "import torch; assert torch.cuda.is_available(), 'CUDA not available'" || (echo "CUDA support in torch, missing it is." && exit 1)
- ctest --test-dir build -R ExaTrkX
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
Examples/Python/python/acts/examples/reconstruction.py (1)
1795-1800: Document the selected features, hmm yes.Mysterious these feature indices [0, 1, 2] are. Documentation for their meaning, add you must. Spatial coordinates (x, y, z) perhaps they represent?
metricLearningConfig["modelPath"] = str(modelDir / "embed.pt") - metricLearningConfig["selectedFeatures"] = [0, 1, 2] + # Features: [0, 1, 2] represent spatial coordinates (x, y, z) + metricLearningConfig["selectedFeatures"] = [0, 1, 2] filterConfig["modelPath"] = str(modelDir / "filter.pt") - filterConfig["selectedFeatures"] = [0, 1, 2] + # Features: [0, 1, 2] represent spatial coordinates (x, y, z) + filterConfig["selectedFeatures"] = [0, 1, 2] gnnConfig["modelPath"] = str(modelDir / "gnn.pt") gnnConfig["undirected"] = True - gnnConfig["selectedFeatures"] = [0, 1, 2] + # Features: [0, 1, 2] represent spatial coordinates (x, y, z) + gnnConfig["selectedFeatures"] = [0, 1, 2]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Examples/Python/python/acts/examples/reconstruction.py(3 hunks)
🔇 Additional comments (3)
Examples/Python/python/acts/examples/reconstruction.py (3)
1832-1839: Approve the new PrototracksToTracks algorithm, I do.
Wisely implemented, this conversion from prototracks to tracks is. The force is strong with this one.
1843-1843: Consistent track input/output naming, maintain we must.
Updated track references in matching configuration, I see. Consistency in the force, this brings.
Also applies to: 1862-1863
Line range hint 1832-1863: Verify the track conversion workflow, we shall.
The path from prototracks to tracks, verify we must. Ensure proper flow of the force through all components.
✅ Verification successful
Verified the track conversion workflow is, hmmmm.
Through the Force, seen I have that proper implementation exists:
- The
PrototracksToTracksalgorithm, correctly converts prototracks to tracks it does, usingVectorTrackContainerandVectorMultiTrajectory - The track matching through
TrackTruthMatcher, properly handles the converted tracks it does - The track state container, managed correctly throughout the workflow it is
A balanced flow of the Force I sense, from prototracks to tracks, and through the matching process. No disturbances in the implementation detected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the track conversion workflow
# Check for proper track handling in the codebase
echo "Searching for track conversion usage..."
rg -A 5 "PrototracksToTracks"
# Check for track matching configurations
echo "Checking track matching configurations..."
rg "inputTracks.*=.*[\"']tracks[\"'].*TrackTruthMatcher"
Length of output: 4565
Script:
#!/bin/bash
# Deeper investigation of the track conversion workflow, we must perform
# Check the implementation of PrototracksToTracks, we shall
echo "Implementation of PrototracksToTracks, examining..."
ast-grep --pattern 'ProcessCode PrototracksToTracks::execute($_) {
$$$
}'
# Search for track container usage in the codebase, we must
echo "Track container handling, investigating..."
rg -A 5 "VectorTrackContainer"
# The track matching implementation, inspect we shall
echo "Track matching implementation, searching..."
ast-grep --pattern 'ProcessCode TrackTruthMatcher::execute($_) {
$$$
}'
Length of output: 73212
Script:
#!/bin/bash
# The implementation of TrackTruthMatcher, find we must
echo "Track truth matching implementation, seeking..."
rg -A 10 "TrackTruthMatcher::execute"
# The prototrack conversion details, examine we shall
echo "Prototrack conversion details, investigating..."
rg -A 10 "PrototracksToTracks::execute"
# The track state handling, understand we must
echo "Track state handling, searching..."
rg -A 5 "trackStateContainer"
Length of output: 41415
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
Examples/Python/python/acts/examples/reconstruction.py (1)
1826-1833: A new algorithm for converting prototracks to tracks, added it is.The force is strong with this change. A dedicated algorithm for converting prototracks to tracks, implemented it has been. Clear and focused, the implementation is.
Consider adding error handling for cases where measurements are missing or invalid, you should.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Examples/Python/python/acts/examples/reconstruction.py(3 hunks)
🔇 Additional comments (2)
Examples/Python/python/acts/examples/reconstruction.py (2)
1789-1794: Hmmmm, feature selection configuration for Torch backend, I see.
Correctly configured, the feature selection is. Three spatial coordinates [0,1,2] for each component, selected they are.
1837-1837: Updated track references in matching and performance components, I observe.
Consistent with the new track conversion flow, these changes are. The track matching and performance components now use the converted tracks instead of prototracks, they do.
Verify the track references, we must:
Also applies to: 1856-1857
✅ Verification successful
Let me examine the actual usage of these references to determine if they are legitimate utility functions or need updating, hmm.
Correct, the review comment is, hmm. Legitimate changes to track references, these are.
The references to prototracks in the codebase, legitimate they are. A clear conversion flow from prototracks to tracks exists, yes. The utility classes PrototracksToTracks, PrototracksToSeeds, and others, handle these conversions they do. The change in track references from prototracks to tracks in the matching and performance components, consistent with this design it is.
In the code, see we do:
- Prototracks created from various sources (truth tracking, ExaTrkX)
- Conversion to tracks through
PrototracksToTracksalgorithm - Track matching and performance components using the converted tracks
No issues found in the codebase, there are. The natural flow of track processing, this change follows.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to prototracks that should use tracks
rg -l 'prototracks' | grep -v 'reconstruction.py'
Length of output: 1315
Script:
#!/bin/bash
# Search for actual usage of prototracks in these files to understand the context
rg "prototracks" -A 3 -B 3
Length of output: 28434
|
These tests should work even though cugraph is not available anymore...
Summary by CodeRabbit
New Features
Bug Fixes
Chores