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: unbiased trackstates for fitters that do not smooth #2999

Merged
merged 11 commits into from
Mar 4, 2024

Conversation

AJPfleger
Copy link
Contributor

@AJPfleger AJPfleger commented Feb 28, 2024

What?

Lets the RootTrackStatesWriter.cpp write unbiased states, even if there are no smoothed states.

Why?

The states we are writing are based upon the Kalman Filter formalism. There, we would expect predicted, filtered, and smoothed states. The smoothed states should be the final states. Other fitters, like the GX2F, do not take this path. Therefore, the final estimate is written to predicted.

Until now, the unbiased states could only be computed from smoothed states. However, it is also for interest for other fitters to investigate the unbiased pulls and residuals.

The idea for this PR evolved while investigating the track states for the GX2F (PR #2949). Seems to be useful to have this in.

Notes

We need to check for state.hasCalibrated(), since the physmon tests for the GSF were crashing. It seems, the GSF creates states without measurements. There, the programme aborted with

state.calibratedSize(): 4294967295
template_switch<Fn, 6, 6>(v=4294967295) is not valid (v > NMAX)

@AJPfleger AJPfleger added this to the next milestone Feb 28, 2024
@github-actions github-actions bot added the Component - Examples Affects the Examples module label Feb 28, 2024
Copy link

codecov bot commented Feb 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.67%. Comparing base (6fd0337) to head (a1a038b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2999   +/-   ##
=======================================
  Coverage   48.67%   48.67%           
=======================================
  Files         493      493           
  Lines       29004    29004           
  Branches    13816    13816           
=======================================
  Hits        14117    14117           
  Misses       4947     4947           
  Partials     9940     9940           

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

@AJPfleger AJPfleger marked this pull request as ready for review February 28, 2024 15:52
@AJPfleger AJPfleger added the 🚧 WIP Work-in-progress label Feb 28, 2024
@AJPfleger AJPfleger marked this pull request as draft February 29, 2024 10:08
@AJPfleger AJPfleger marked this pull request as ready for review February 29, 2024 12:39
@AJPfleger AJPfleger removed the 🚧 WIP Work-in-progress label Feb 29, 2024
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.

I wonder if it would be better to actually write smoothed parameters in the GX2 as smoothed corresponds to a global fit.

In case of the GSF there is a smoothing step which seems not to be taken in one of the tests since the output changes?

@AJPfleger
Copy link
Contributor Author

We had about 1,5 years ago a discussion in that direction. Since non of the 3 categories really make sense for the GX2F, we settled to use predicted since, it is the least confusing for:

  • There is no filtering and smoothing happening
  • It is the first in the list and people might look for the missing predicted/filtered if smoothed is chosen
  • In the KF thefiltered andsmoothed is computed from the predicted/filtered instead of only the calibrated measurements. This would change how the categories depend on each other (in a logical sense)

@andiwand
Copy link
Contributor

andiwand commented Mar 1, 2024

to me predicted seems the most confusing one but okay I don't want to open a discussion on this here

But I don't think these changes should affect the GSF output

@paulgessinger
Copy link
Member

paulgessinger commented Mar 1, 2024

@andiwand Not sure this helps, but originally, I was thinking that we would treat them stacked, like

  1. predicted
  2. filtered
  3. smoothed

There's an accessor that returns the highest of these, i.e. smoothed if available, otherwise filtered if available, otherwise predicted. That accessor is called parameters (although that might have turned out to be confusing now).

Anyway, I don't have a strong preference what the GX2F should write its result in. I could also see them being conceptually closest to smoothed, rather than predicted.

@AJPfleger AJPfleger added the 🚧 WIP Work-in-progress label Mar 4, 2024
@AJPfleger AJPfleger requested a review from andiwand March 4, 2024 14:40
@AJPfleger AJPfleger removed the 🚧 WIP Work-in-progress label Mar 4, 2024
@kodiakhq kodiakhq bot merged commit 9fcfb7e into acts-project:main Mar 4, 2024
54 checks passed
@github-actions github-actions bot removed the automerge label Mar 4, 2024
@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Mar 4, 2024
@AJPfleger
Copy link
Contributor Author

@paulgessinger in developer meeting: should we refactor to automatically take the "highest" fit?

@paulgessinger paulgessinger modified the milestones: next, v33.0.0 Mar 6, 2024
@paulgessinger paulgessinger removed the Breaks Athena build This PR breaks the Athena build label Mar 6, 2024
Ragansu pushed a commit to Ragansu/acts that referenced this pull request Mar 7, 2024
…ct#2999)

## What?
Lets the `RootTrackStatesWriter.cpp` write unbiased states, even if there are no `smoothed` states.

## Why?
The states we are writing are based upon the Kalman Filter formalism. There, we would expect `predicted`, `filtered`, and `smoothed` states. The `smoothed` states should be the final states. Other fitters, like the GX2F, do not take this path. Therefore, the final estimate is written to `predicted`.

Until now, the `unbiased` states could only be computed from `smoothed` states. However, it is also for interest for other fitters to investigate the `unbiased` pulls and residuals.

The idea for this PR evolved while investigating the track states for the GX2F (PR acts-project#2949). Seems to be useful to have this in.

## Notes
We need to check for `state.hasCalibrated()`, since the physmon tests for the GSF were crashing. It seems, the GSF creates states without measurements. There, the programme aborted with
```
state.calibratedSize(): 4294967295
template_switch<Fn, 6, 6>(v=4294967295) is not valid (v > NMAX)
```
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 6, 2024
…ct#2999)

## What?
Lets the `RootTrackStatesWriter.cpp` write unbiased states, even if there are no `smoothed` states.

## Why?
The states we are writing are based upon the Kalman Filter formalism. There, we would expect `predicted`, `filtered`, and `smoothed` states. The `smoothed` states should be the final states. Other fitters, like the GX2F, do not take this path. Therefore, the final estimate is written to `predicted`.

Until now, the `unbiased` states could only be computed from `smoothed` states. However, it is also for interest for other fitters to investigate the `unbiased` pulls and residuals.

The idea for this PR evolved while investigating the track states for the GX2F (PR acts-project#2949). Seems to be useful to have this in.

## Notes
We need to check for `state.hasCalibrated()`, since the physmon tests for the GSF were crashing. It seems, the GSF creates states without measurements. There, the programme aborted with
```
state.calibratedSize(): 4294967295
template_switch<Fn, 6, 6>(v=4294967295) is not valid (v > NMAX)
```
asalzburger pushed a commit to asalzburger/acts that referenced this pull request May 21, 2024
…ct#2999)

## What?
Lets the `RootTrackStatesWriter.cpp` write unbiased states, even if there are no `smoothed` states.

## Why?
The states we are writing are based upon the Kalman Filter formalism. There, we would expect `predicted`, `filtered`, and `smoothed` states. The `smoothed` states should be the final states. Other fitters, like the GX2F, do not take this path. Therefore, the final estimate is written to `predicted`.

Until now, the `unbiased` states could only be computed from `smoothed` states. However, it is also for interest for other fitters to investigate the `unbiased` pulls and residuals.

The idea for this PR evolved while investigating the track states for the GX2F (PR acts-project#2949). Seems to be useful to have this in.

## Notes
We need to check for `state.hasCalibrated()`, since the physmon tests for the GSF were crashing. It seems, the GSF creates states without measurements. There, the programme aborted with
```
state.calibratedSize(): 4294967295
template_switch<Fn, 6, 6>(v=4294967295) is not valid (v > NMAX)
```
@AJPfleger AJPfleger deleted the trackstates branch September 3, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants