Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(gx2f): Add new error NotEnoughMeasurements #2981

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

AJPfleger
Copy link
Contributor

@AJPfleger AJPfleger commented 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:

(std::abs(chi2sum / oldChi2sum - 1) <

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.

Copy link
Contributor

@andiwand andiwand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me 👍

Copy link

codecov bot commented Feb 22, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (e9ab1f7) 48.74% compared to head (be2c8b2) 48.74%.
Report is 1 commits behind head on main.

Files Patch % Lines
...nclude/Acts/TrackFitting/GlobalChiSquareFitter.hpp 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2981      +/-   ##
==========================================
- Coverage   48.74%   48.74%   -0.01%     
==========================================
  Files         493      493              
  Lines       28918    28926       +8     
  Branches    13760    13765       +5     
==========================================
+ Hits        14096    14099       +3     
- Misses       4921     4924       +3     
- Partials     9901     9903       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kodiakhq kodiakhq bot merged commit 15ace85 into acts-project:main Feb 22, 2024
57 checks passed
@AJPfleger AJPfleger deleted the not-enough-meas branch February 22, 2024 11:41
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Feb 22, 2024
kodiakhq bot pushed a commit that referenced this pull request Feb 22, 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:
- #2972
- #2981
kodiakhq bot pushed a commit that referenced this pull request Feb 29, 2024
I forgot to add the error message in PR #2981.
@paulgessinger paulgessinger modified the milestones: next, v33.0.0 Mar 6, 2024
@paulgessinger paulgessinger removed the Fails Athena tests This PR causes a failure in the Athena tests label Mar 6, 2024
Ragansu pushed a commit to Ragansu/acts that referenced this pull request Mar 7, 2024
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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Core Affects the Core module Track Fitting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants