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

fix: Fitting with ouliers #804

Merged
merged 4 commits into from
May 21, 2021

Conversation

Corentin-Allaire
Copy link
Contributor

Fix an issue with the fitting when there are outliers.

Since the outliers use to not have filtered parameters and covariance, when going through the different states for the smoothing their covariance matrix was taken to be equal to 0 resulting in a bad smoothing. Instead we now put the filtered covariance for the outliers equal to the predicted one (like the material surface).

The material flag was also always on for the measurement state now we check that material is present to turn that flag on.

@Corentin-Allaire Corentin-Allaire added this to the next milestone May 18, 2021
@Corentin-Allaire Corentin-Allaire added Bug Something isn't working Component - Core Affects the Core module Impact - Major Significant bug and/or affects a lot of modules labels May 18, 2021
@codecov
Copy link

codecov bot commented May 18, 2021

Codecov Report

Merging #804 (b11e085) into main (7b9f78c) will increase coverage by 0.00%.
The diff coverage is 33.33%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #804   +/-   ##
=======================================
  Coverage   48.64%   48.64%           
=======================================
  Files         328      328           
  Lines       16898    16900    +2     
  Branches     7934     7935    +1     
=======================================
+ Hits         8220     8221    +1     
  Misses       3080     3080           
- Partials     5598     5599    +1     
Impacted Files Coverage Δ
Core/include/Acts/TrackFitting/KalmanFitter.hpp 39.60% <33.33%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b9f78c...b11e085. Read the comment docs.

@asalzburger asalzburger self-requested a review May 21, 2021 07:05
Copy link
Contributor

@asalzburger asalzburger left a comment

Choose a reason for hiding this comment

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

Makes totally sense, I will have a look if this is an issue with the CKF as well ...

@asalzburger asalzburger merged commit 8f0f37a into acts-project:main May 21, 2021
@asalzburger
Copy link
Contributor

Just to add: the CKF seems to have this already available:

      if (isOutlier) {
        ACTS_VERBOSE("Creating outlier track state with tip = " << currentTip);
        // Set the outlier flag
        typeFlags.set(TrackStateFlag::OutlierFlag);
        // Increment number of outliers
        tipState.nOutliers++;
        // No Kalman update for outlier
        // Set the filtered parameter index to be the same with predicted
        // parameter
        trackStateProxy.data().ifiltered = trackStateProxy.data().ipredicted;

@paulgessinger paulgessinger modified the milestones: next, v8.3.0 May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Component - Core Affects the Core module Impact - Major Significant bug and/or affects a lot of modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants