fix: Change the component reducer for the GSF (to fix navigation issues) #3521
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andiwand
reviewed
Aug 20, 2024
github-actions
bot
added
Component - Core
github-actions
bot
added
Infrastructure
This was referenced Aug 20, 2024
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
andiwand
approved these changes
Aug 21, 2024
|
acts-project-service
added
the
Breaks Athena build
kodiakhq bot
pushed a commit
that referenced
this pull request
Sep 1, 2024
…3442) I think this is one of the big navigation pitfalls. If we end up in the wrong volume the navigation is practically lost. This is too expensive to check in production so I opted for an `assert`. Not sure if that is a great solution since users might try to measure performance with non release build by accident. But that was the only way I could think of. blocked by - #3521 - #3481
This was referenced Oct 1, 2024
kodiakhq bot
pushed a commit
that referenced
this pull request
Oct 1, 2024
After #3521 we also want to change the default and remove the weighted mean reduction as it is unstable. This pull request primarily focuses on removing the `WeightedComponentReducerLoop` struct and its associated functionality from the `MultiEigenStepperLoop` class and its tests. The changes simplify the codebase by eliminating the weighted component reduction logic and replacing it with the `MaxWeightReducerLoop`. ### Removal of `WeightedComponentReducerLoop`: * [`Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp`](diffhunk://#diff-2ec4ab79520c414c4eb526d00ede45a6a10f8eeba06398d81c315f9be5686e9eL48-L145): Removed the `WeightedComponentReducerLoop` struct and its methods, which were responsible for reducing the multicomponent state by summing weighted values. ### Replacement with `MaxWeightReducerLoop`: * [`Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp`](diffhunk://#diff-2ec4ab79520c414c4eb526d00ede45a6a10f8eeba06398d81c315f9be5686e9eL243-R145): Updated the `MultiEigenStepperLoop` class to use `MaxWeightReducerLoop` instead of `WeightedComponentReducerLoop` as the default component reducer. ### Test updates: * [`Tests/UnitTests/Core/Propagator/MultiStepperTests.cpp`](diffhunk://#diff-35cd1eb9d226dec9f6017cd99b5605c2da3bbaa4c3f9ffbe5b0bed7efcfdc0ddL141-L165): Removed the test case for `WeightedComponentReducerLoop`, which validated the position and direction calculations using weighted sums.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the used component reducer in the examples framework. With the next major release, the default reducer should also be changed in Core.
Also fixes a bug, as the
MaxMomentumReducerLoopwas in fact aMinMomentumReducerLoop.