ci(gx2f): add physmon truth tracking for the GX2F #2966
Merged
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
github-actions
bot
added
Infrastructure
📊: Physics performance monitoring for dcc4472physmon summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2966 +/- ##
=======================================
Coverage 48.74% 48.74%
=======================================
Files 493 493
Lines 28926 28926
Branches 13765 13765
=======================================
Hits 14099 14099
Misses 4924 4924
Partials 9903 9903 ☔ View full report in Codecov by Sentry. |
andiwand
previously approved these changes
Feb 21, 2024
AJPfleger
added
🛑 blocked
kodiakhq bot
pushed a commit
that referenced
this pull request
Feb 22, 2024
## What? Returns an error, in case the number of measurements is too low for a fit. This check takes into account the evaluated dimensions of the measurements. To fit, we need at least NDF+1 measurements. However, we n-dimensional measurements count for n measurements, reducing the effective number of needed measurements. We might encounter the case, where we cannot use some (parts of a) measurements, maybe if we do not support that kind of measurement. This is also taken into account here. `ndf = 4` is chosen, since this a minimum that makes sense for us, but a more general approach is desired. ## Why? We had in PR #2966 FPE Problems due to a division by zero in: https://github.com/acts-project/acts/blob/a42f23b96876a0fdfba5a8ab1b6c90a9b1f2dc30/Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp#L716 This happened, when no measurements were evaluated by the collector. It could happen that we didn't collect any measurements, since the ODD uses also some 3-dimensional measurements. Currently, the GX2F can only handle 1- and 2-dimensional measurements. ## Future TODO Make `ndf` dependent on the problem. For this we need to find a way to deduce, how many parameters we want to fit.
github-actions
bot
removed
Component - Core
kodiakhq bot
pushed a commit
that referenced
this pull request
Feb 22, 2024
## Issue I figured out, that the trackstates in the final track contain the wrong covariance. This occurred, while investigating the unbiased pulls. There, some dimensions had a negative covariance. https://github.com/acts-project/acts/blob/19464fa0d24f031083ef3ad3108b74f2b5fa5454/Examples/Io/Root/src/RootTrackStatesWriter.cpp#L456 ## Reason After the last update we calculate the covariance of the initial parameters and add this covariance to the final track. However, the last propagation was done with the initial guess for the covariance. Therefore, the wrong covariance got propagated and assigned to each trackstate. ## Solution Propagate an additional time, with the final parameters+covariance. ## Future Plans Since an additional propagation is quite expensive, we should look into the following two ideas: - calculate covariance after each update (matrix inverse) - toggle the re-propagation if we are not interested in the propagated covariance ## Notes This also removes `nUpdate` from the Actor since it wasn't needed anymore. ## Blocked by - #2972 - #2966
EleniXoch
pushed a commit
to EleniXoch/acts
that referenced
this pull request
May 6, 2024
## What? Returns an error, in case the number of measurements is too low for a fit. This check takes into account the evaluated dimensions of the measurements. To fit, we need at least NDF+1 measurements. However, we n-dimensional measurements count for n measurements, reducing the effective number of needed measurements. We might encounter the case, where we cannot use some (parts of a) measurements, maybe if we do not support that kind of measurement. This is also taken into account here. `ndf = 4` is chosen, since this a minimum that makes sense for us, but a more general approach is desired. ## Why? We had in PR acts-project#2966 FPE Problems due to a division by zero in: https://github.com/acts-project/acts/blob/a42f23b96876a0fdfba5a8ab1b6c90a9b1f2dc30/Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp#L716 This happened, when no measurements were evaluated by the collector. It could happen that we didn't collect any measurements, since the ODD uses also some 3-dimensional measurements. Currently, the GX2F can only handle 1- and 2-dimensional measurements. ## Future TODO Make `ndf` dependent on the problem. For this we need to find a way to deduce, how many parameters we want to fit.
EleniXoch
pushed a commit
to EleniXoch/acts
that referenced
this pull request
May 6, 2024
This PR adds the GX2F to the physmon. It runs stable even though the ODD uses 3D-measurements for the inner pixels. So far the GX2F supports only 1-2D measurements. blocked by: - acts-project#2972 - acts-project#2981
EleniXoch
pushed a commit
to EleniXoch/acts
that referenced
this pull request
May 6, 2024
) ## Issue I figured out, that the trackstates in the final track contain the wrong covariance. This occurred, while investigating the unbiased pulls. There, some dimensions had a negative covariance. https://github.com/acts-project/acts/blob/19464fa0d24f031083ef3ad3108b74f2b5fa5454/Examples/Io/Root/src/RootTrackStatesWriter.cpp#L456 ## Reason After the last update we calculate the covariance of the initial parameters and add this covariance to the final track. However, the last propagation was done with the initial guess for the covariance. Therefore, the wrong covariance got propagated and assigned to each trackstate. ## Solution Propagate an additional time, with the final parameters+covariance. ## Future Plans Since an additional propagation is quite expensive, we should look into the following two ideas: - calculate covariance after each update (matrix inverse) - toggle the re-propagation if we are not interested in the propagated covariance ## Notes This also removes `nUpdate` from the Actor since it wasn't needed anymore. ## Blocked by - acts-project#2972 - acts-project#2966
asalzburger
pushed a commit
to asalzburger/acts
that referenced
this pull request
May 21, 2024
## What? Returns an error, in case the number of measurements is too low for a fit. This check takes into account the evaluated dimensions of the measurements. To fit, we need at least NDF+1 measurements. However, we n-dimensional measurements count for n measurements, reducing the effective number of needed measurements. We might encounter the case, where we cannot use some (parts of a) measurements, maybe if we do not support that kind of measurement. This is also taken into account here. `ndf = 4` is chosen, since this a minimum that makes sense for us, but a more general approach is desired. ## Why? We had in PR acts-project#2966 FPE Problems due to a division by zero in: https://github.com/acts-project/acts/blob/a42f23b96876a0fdfba5a8ab1b6c90a9b1f2dc30/Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp#L716 This happened, when no measurements were evaluated by the collector. It could happen that we didn't collect any measurements, since the ODD uses also some 3-dimensional measurements. Currently, the GX2F can only handle 1- and 2-dimensional measurements. ## Future TODO Make `ndf` dependent on the problem. For this we need to find a way to deduce, how many parameters we want to fit.
asalzburger
pushed a commit
to asalzburger/acts
that referenced
this pull request
May 21, 2024
This PR adds the GX2F to the physmon. It runs stable even though the ODD uses 3D-measurements for the inner pixels. So far the GX2F supports only 1-2D measurements. blocked by: - acts-project#2972 - acts-project#2981
asalzburger
pushed a commit
to asalzburger/acts
that referenced
this pull request
May 21, 2024
) ## Issue I figured out, that the trackstates in the final track contain the wrong covariance. This occurred, while investigating the unbiased pulls. There, some dimensions had a negative covariance. https://github.com/acts-project/acts/blob/19464fa0d24f031083ef3ad3108b74f2b5fa5454/Examples/Io/Root/src/RootTrackStatesWriter.cpp#L456 ## Reason After the last update we calculate the covariance of the initial parameters and add this covariance to the final track. However, the last propagation was done with the initial guess for the covariance. Therefore, the wrong covariance got propagated and assigned to each trackstate. ## Solution Propagate an additional time, with the final parameters+covariance. ## Future Plans Since an additional propagation is quite expensive, we should look into the following two ideas: - calculate covariance after each update (matrix inverse) - toggle the re-propagation if we are not interested in the propagated covariance ## Notes This also removes `nUpdate` from the Actor since it wasn't needed anymore. ## Blocked by - acts-project#2972 - acts-project#2966
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 PR adds the GX2F to the physmon. It runs stable even though the ODD uses 3D-measurements for the inner pixels. So far the GX2F supports only 1-2D measurements.
blocked by:
NotEnoughMeasurements#2981