feat: Helpers to calculate residuals, chi2 and unbiased states #3877
Conversation
WalkthroughChanges made to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TrackHelpers
participant RootTrackStatesWriter
User->>RootTrackStatesWriter: Request track parameters
RootTrackStatesWriter->>TrackHelpers: calculateUnbiasedParametersCovariance(state)
TrackHelpers-->>RootTrackStatesWriter: Return unbiased parameters
RootTrackStatesWriter-->>User: Provide track parameters
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 (
|
github-actions
bot
added
Component - Core
|
Sonar has a couple of valid points on the usage of |
@paulgessinger I think this is the code I copied from other places to do the |
|
@andiwand This only complains about cases where the template parameter is subsequently accessed. Indeed, this only became possible in C++20 with explicit template parameters to lambdas. Ultimately, we should migrate to it, but I personally think updating this incrementally is fine. |
acts-policybot
bot
dismissed
pbutti’s stale review
Invalidated by push of 5e86218
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
Core/include/Acts/Utilities/TrackHelpers.hpp (1)
508-534: Refactor common code, you canThe functions
calculatePredictedResidual,calculateFilteredResidual, andcalculateSmoothedResidualmuch code they share. Improve maintainability, to refactor common logic into a single helper function, consider you should.Also applies to: 544-570, 580-606
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
Core/include/Acts/Utilities/TrackHelpers.hpp(2 hunks)Examples/Io/Root/src/RootTrackStatesWriter.cpp(2 hunks)
🔇 Additional comments (3)
Core/include/Acts/Utilities/TrackHelpers.hpp (1)
621-629: Template parameters in lambdas, ensure compatibility you must
In your use of visit_measurement, auto parameters in lambdas you use. Compatibility issues this may cause with C++ standards before C++20. Explicit template parameters, prefer you might to ensure broader compatibility.
Also applies to: 644-652, 667-675
Examples/Io/Root/src/RootTrackStatesWriter.cpp (2)
19-19: Include necessary header, you have.
Adding "Acts/Utilities/TrackHelpers.hpp" ensures access to required helper functions.
484-486: Simplify code with helper function, you do.
Replacing complex calculations with Acts::calculateUnbiasedParametersCovariance(state) enhances readability and maintainability.
|
acts-project-service
added
the
Breaks Athena build
This came up a couple of times and it really should be in core so I added helpers to
For now I added them to
TrackHelpers.hppbut potentially this could also live in theTrackStateProxy.Summary by CodeRabbit
New Features
Bug Fixes
Documentation