acts-project
refactor: Move MLAmbiguitySolver to Core
This PR moves the MLAmbiguitySolver to Core, this will allow us to test it more easily with ATLAS in the future. It also removes the DBScan version of this algorithm as it was shown to be way less effective.
📊: Physics performance monitoring for a3555d02246521e2da7d72dc11fe384e458d0a82
physmon summary
- ✅ Particles fatras
- ✅ Particles geant4
- ✅ Particles ttbar
- ✅ Vertices ttbar
- ✅ Truth tracking (KF)
- ✅ Truth tracking (GSF)
- ✅ Truth tracking (GX2F)
- ✅ Truth tracking (KF refit)
- ✅ Truth tracking (GSF refit)
- ✅ CKF finding performance | trackfinding | single muon | truth smeared seeding
- ✅ CKF fitting performance | trackfinding | single muon | truth smeared seeding
- ✅ CKF track summary | trackfinding | single muon | truth smeared seeding
- ✅ Seeding trackfinding | single muon | truth estimated seeding
- ✅ CKF finding performance | trackfinding | single muon | truth estimated seeding
- ✅ CKF fitting performance | trackfinding | single muon | truth estimated seeding
- ✅ CKF track summary | trackfinding | single muon | truth estimated seeding
- ✅ Seeding trackfinding | single muon | default seeding
- ✅ CKF finding performance | trackfinding | single muon | default seeding
- ✅ CKF fitting performance | trackfinding | single muon | default seeding
- ✅ CKF track summary | trackfinding | single muon | default seeding
- ✅ Seeding trackfinding | single muon | orthogonal seeding
- ✅ CKF finding performance | trackfinding | single muon | orthogonal seeding
- ✅ CKF fitting performance | trackfinding | single muon | orthogonal seeding
- ✅ CKF track summary | trackfinding | single muon | orthogonal seeding
- ✅ Seeding trackfinding | 4 muon x 50 vertices | default seeding
- ✅ CKF finding performance | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ CKF fitting performance | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ CKF track summary | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ Ambisolver finding performance | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ IVF notime | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ AMVF gauss notime | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ AMVF grid time | trackfinding | 4 muon x 50 vertices | default seeding
- ✅ Seeding trackfinding | ttbar with 200 pileup | default seeding
- ✅ CKF finding performance | trackfinding | ttbar with 200 pileup | default seeding
- ✅ CKF fitting performance | trackfinding | ttbar with 200 pileup | default seeding
- ✅ CKF track summary | trackfinding | ttbar with 200 pileup | default seeding
- ✅ Ambisolver finding performance | trackfinding | ttbar with 200 pileup | default seeding
- ✅ ML Ambisolver | trackfinding | ttbar with 200 pileup | default seeding
- ✅ AMVF gauss notime | trackfinding | ttbar with 200 pileup | default seeding
- ✅ AMVF grid time | trackfinding | ttbar with 200 pileup | default seeding
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 47.46%. Comparing base (
6b2e205) to head (da5a458). Report is 179 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #3272 +/- ##
=======================================
Coverage 47.46% 47.46%
=======================================
Files 510 510
Lines 30198 30198
Branches 14639 14639
=======================================
Hits 14332 14332
Misses 5335 5335
Partials 10531 10531
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
4.9% Duplication on New Code
See analysis details on SonarCloud
![sonarqubecloud[bot] avatar](https://avatars.githubusercontent.com/in/12526?v=4 "Published by a pub.dev verified publisher"){kind=small}
Walkthrough
Enhancements made to performance monitoring scripts and ambiguity resolution algorithms in the ACTS project. The phys_perf_mon.sh script now checks for a specific file to trigger performance comparisons, while the physmon_trackfinding_ttbar_pu200.py script introduces a new machine learning function for ambiguity resolution. New header files define concepts and classes for machine learning-based track finding, and several obsolete configurations have been removed to streamline the codebase. Overall, these changes improve the functionality and clarity of the performance monitoring and ambiguity resolution processes.
Changes
| File | Change Summary |
|---|---|
CI/physmon/phys_perf_mon.sh |
Added conditional block in trackfinding function to check for performance_finding_ckf_ml_solver.root and execute run_histcmp. |
CI/physmon/workflows/physmon_trackfinding_ttbar_pu200.py |
Introduced addAmbiguityResolutionML function for ML-based ambiguity resolution; updated addAmbiguityResolution to use ckf_tracks. Renamed output files for ML performance metrics. |
Core/include/Acts/AmbiguityResolution/AmbiguityNetworkConcept.hpp |
Added AmbiguityNetworkConcept and DummyTrackContainer type alias. |
Core/include/Acts/AmbiguityResolution/AmbiguityResolutionML.hpp |
Introduced AmbiguityResolutionML class with methods for track hits mapping and ambiguity solving. |
Core/include/Acts/TrackFinding/detail/AmbiguityTrackClustering.hpp |
Updated documentation for clusterDuplicateTracks function. |
Examples/Algorithms/TrackFindingML/CMakeLists.txt |
Removed src/AmbiguityResolutionML.cpp and src/AmbiguityResolutionMLDBScanAlgorithm.cpp from build sources. |
Examples/Algorithms/TrackFindingML/include/ActsExamples/TrackFindingML/AmbiguityDBScanClustering.hpp |
Deleted file containing dbscanTrackClustering function. |
Examples/Algorithms/TrackFindingML/include/ActsExamples/TrackFindingML/AmbiguityResolutionML.hpp |
Deleted file defining AmbiguityResolutionML class. |
Examples/Algorithms/TrackFindingML/include/ActsExamples/TrackFindingML/AmbiguityResolutionMLAlgorithm.hpp |
Updated class to inherit from IAlgorithm and modified Config struct. |
Examples/Algorithms/TrackFindingML/include/ActsExamples/TrackFindingML/AmbiguityResolutionMLDBScanAlgorithm.hpp |
Deleted file defining AmbiguityResolutionMLDBScanAlgorithm class. |
Examples/Algorithms/TrackFindingML/include/ActsExamples/TrackFindingML/SeedFilterMLAlgorithm.hpp |
Updated header inclusions without changing class logic. |
Examples/Algorithms/TrackFindingML/src/AmbiguityResolutionML.cpp |
Deleted implementation of AmbiguityResolutionML class. |
Examples/Algorithms/TrackFindingML/src/AmbiguityResolutionMLAlgorithm.cpp |
Updated constructor and internal logic for AmbiguityResolutionMLAlgorithm. |
Examples/Algorithms/TrackFindingML/src/AmbiguityResolutionMLDBScanAlgorithm.cpp |
Deleted implementation of AmbiguityResolutionMLDBScanAlgorithm. |
Examples/Io/Csv/include/ActsExamples/Io/Csv/CsvSpacePointWriter.hpp |
Corrected class name casing for CsvSpacePointWriter. |
Examples/Io/Csv/src/CsvSpacePointWriter.cpp |
Updated class name and method signatures for consistency. |
Examples/Python/python/acts/examples/reconstruction.py |
Removed AmbiguityResolutionMLDBScanConfig and updated ambiguity resolution functions. |
Examples/Python/src/Onnx.cpp |
Removed AmbiguityResolutionMLDBScanAlgorithm from Python bindings. |
Examples/Python/src/Output.cpp |
Updated CSV writer class naming conventions. |
Examples/Scripts/Python/MLAmbiguityResolution/ambiguity_solver_network.py |
Modified sorting logic in prepareDataSet function. |
Plugins/Onnx/include/Acts/Plugins/Onnx/AmbiguityTrackClassifier.hpp |
Removed solveAmbiguity method from AmbiguityTrackClassifier. |
Core/src/TrackFinding/AmbiguityTrackClustering.cpp |
Changed iteration method in clusterDuplicateTracks to reverse iteration. |
Possibly related PRs
- #3807: Enhances logging capabilities in the
GbtsTrackingFilterandSeedFinderGbtsclasses, which may relate to performance monitoring aspects in the main PR's modifications to thephys_perf_mon.shscript. - #3812: This PR introduces enhancements for ambiguity resolution using machine learning, which aligns with the main PR's focus on performance monitoring related to machine learning outputs.
- #3836: The refactor of the
HitSelectorclass to allow for more precise hit selection could relate to the performance monitoring enhancements in the main PR, as both involve improving data handling and processing. - #3907: The changes in this PR to allow measurement setting without extra initialization could be relevant to the main PR's focus on performance metrics, as both involve optimizing processes related to measurements.
- #3921: The compatibility updates for EDM4hep versions may indirectly relate to the main PR's performance monitoring enhancements, as both involve ensuring that the system can handle various data formats effectively.
- #3950: The relocation of spline functionality to a more accessible location in the visualization components could relate to the main PR's enhancements in performance monitoring, as both involve improving the usability and functionality of the codebase.
Suggested reviewers
- paulgessinger
- andiwand
In the code, changes abound,
Performance metrics now profound.
Ambiguities resolved with care,
Machine learning's magic we share.
Yoda's wisdom guides the way,
In track finding, we shall sway! ✨
📜 Recent review details
Configuration used: CodeRabbit UI Review profile: CHILL
📥 Commits
Reviewing files that changed from the base of the PR and between dbe4a8134d63079bf0185b71fdfab71b4b0646a9 and a3555d02246521e2da7d72dc11fe384e458d0a82.
📒 Files selected for processing (2)
Examples/Python/python/acts/examples/reconstruction.py(4 hunks)Examples/Python/src/Output.cpp(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Examples/Python/src/Output.cpp
🔇 Additional comments (3)
Examples/Python/python/acts/examples/reconstruction.py (3)
1969-1984: Hmm, proper track truth matching configuration, I see.
Correctly configured, the track truth matcher is. Double matching enabled, it has. Proper aliases for track-particle matching, established they are.
1992-1996: Wise configuration of track writers, this is.
The essential parameters for track writing, properly set they are. Summary, states, performance metrics - all in balance, they remain.
Line range hint 2009-2079: Strong with the Force, this ML ambiguity resolution implementation is.
A two-stage approach, implemented it has:
- ML-based initial resolution with ONNX model
- Greedy resolution for final refinement
Yet, verify the model path we must.
✅ Verification successful
Hmm, verify the ONNX model path, we must not. Optional parameter it is, and dynamic at runtime, it shall be provided.
Track matching configuration, properly implemented it is. Follows the established pattern across the codebase, matching algorithm does. Consistent with other truth matching implementations in:
- Kalman filter tracking
- GSF tracking
- CKF tracking
- ExaTrkX tracking
Correct and complete, the implementation is. Trust in the Force, we shall.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify ONNX model existence and track matching configuration
# Check if ONNX model path is provided in any configuration files
rg -l "\.onnx" --type yaml --type json
# Verify track matching configuration
rg "TrackTruthMatcher" -A 5
Length of output: 11932
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?
🪧 Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
I pushed a fix in commit <commit_id>, please review it.Generate unit testing code for this file.Open a follow-up GitHub issue for this discussion.
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. Examples:@coderabbitai generate unit testing code for this file.@coderabbitai modularize this function.
- PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.@coderabbitai read src/utils.ts and generate unit testing code.@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.@coderabbitai help me debug CodeRabbit configuration file.
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)
@coderabbitai pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai resolveresolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai helpto get help.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
CodeRabbit Configuration File (.coderabbit.yaml)
- You can programmatically configure CodeRabbit by adding a
.coderabbit.yamlfile to the root of your repository. - Please see the configuration documentation for more information.
- If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation:
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
Documentation and Community
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
![coderabbitai[bot] avatar](https://avatars.githubusercontent.com/in/347564?v=4 "Published by a pub.dev verified publisher"){kind=small}
@CarloVarni Seems like all of the ttbar outputs change over on GitLab. Not quite sure why it would not affect the GitHub run. Maybe we have another non-reproducibility issue? (@andiwand)
I don't see anything in this PR that would explain this.
@paulgessinger @CarloVarni It seem the issue with the CI bridge is gone ? Could someone reapprove ?
Quality Gate passed
Issues
1 New issue
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
4.5% Duplication on New Code
See analysis details on SonarQube Cloud
