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: Improve full chain pulls by enabling Fatras interactions #2086

Merged
merged 149 commits into from
Jul 24, 2023

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented May 4, 2023

Our full chain pulls are in a bad state. Looks like the reconstruction and simulation energy loss did not match up. This PR switches the Fatras interactions on which should bring our pulls back to standard normal distribution.

Fixes

Blocked by

@andiwand andiwand added the Component - Examples Affects the Examples module label May 4, 2023
@andiwand andiwand added this to the next milestone May 4, 2023
@github-actions
Copy link

github-actions bot commented May 4, 2023

📊 Physics performance monitoring for b24e16e

Summary
Full report
Seeding: seeded, truth estimated, orthogonal
CKF: seeded, truth smeared, truth estimated, orthogonal
IVF: seeded, truth smeared, truth estimated, orthogonal
AMVF: seeded, truth smeared, truth estimated, orthogonal
Ambiguity resolution: seeded, orthogonal
Truth tracking
Truth tracking (GSF)

Vertexing

Vertexing vs. mu
IVF seeded

IVF truth_smeared

IVF truth_estimated

IVF orthogonal

AMVF seeded

AMVF truth_smeared

AMVF truth_estimated

AMVF orthogonal

Seeding

Seeding seeded

Seeding truth_estimated

Seeding orthogonal

CKF

CKF seeded

CKF truth_smeared

CKF truth_estimated

CKF orthogonal

Ambiguity resolution

seeded

Truth tracking (Kalman Filter)

Truth tracking

Truth tracking (GSF)

Truth tracking

@codecov
Copy link

codecov bot commented May 4, 2023

Codecov Report

Merging #2086 (b24e16e) into main (22fa536) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2086   +/-   ##
=======================================
  Coverage   49.51%   49.51%           
=======================================
  Files         450      450           
  Lines       25465    25465           
  Branches    11718    11718           
=======================================
  Hits        12608    12608           
  Misses       4565     4565           
  Partials     8292     8292           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@beomki-yeo
Copy link
Contributor

intialVarInflation should be used to get pulls of 1?

@andiwand
Copy link
Contributor Author

andiwand commented May 5, 2023

intialVarInflation should be used to get pulls of 1?

this is what @pbutti described here #2074

the idea is that the seed is just a start for the fitting but we do not want the fitter to put trust into this start. at the end when you blow up the initial var you let only the measurements speak

@andiwand andiwand added the 🚧 WIP Work-in-progress label May 5, 2023
@andiwand
Copy link
Contributor Author

andiwand commented May 5, 2023

somehow this crashes a bunch of things. need to take a closer look

@andiwand
Copy link
Contributor Author

kodiakhq bot pushed a commit that referenced this pull request Jul 11, 2023
I pulled this out of #2086 since it is not relevant there but I would still like to merge them. These changes are random improvements across the code base which should not have any effects on CPU or physics performance.
@andiwand andiwand changed the title fix: Improve full chain pulls towards standard normal distribution fix: Improve full chain pulls by enabling Fatras interactions Jul 13, 2023
benjaminhuth
benjaminhuth previously approved these changes Jul 24, 2023
Copy link
Member

@benjaminhuth benjaminhuth left a comment

Choose a reason for hiding this comment

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

After discussion in today's stand-up meeting: Let's get that in!

@kodiakhq kodiakhq bot merged commit dbcba62 into acts-project:main Jul 24, 2023
55 checks passed
@andiwand andiwand deleted the fix-full-chain-pulls branch July 24, 2023 14:25
paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Jul 24, 2023
I pulled this out of acts-project#2086 since it is not relevant there but I would still like to merge them. These changes are random improvements across the code base which should not have any effects on CPU or physics performance.
paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Jul 24, 2023
…roject#2086)

Our full chain pulls are in a bad state. Looks like the reconstruction and simulation energy loss did not match up. This PR switches the Fatras interactions on which should bring our pulls back to standard normal distribution.

Fixes
- acts-project#1643

Blocked by
- acts-project#2157
- acts-project#2239
- acts-project#2295
- acts-project#2293
- acts-project#2294
@paulgessinger paulgessinger modified the milestones: next, v28.0.0 Jul 27, 2023
kodiakhq bot pushed a commit that referenced this pull request Jan 24, 2024
An initial track parameter covariance is necessary to avoid double counting and to estimate the covariance purely from the measurements.

Pulled out of #2086 to see the individual effects.

I added this to the ODD and ITk full chain and our physmon

bocked by
- #2829
LaraCalic pushed a commit to LaraCalic/acts that referenced this pull request Feb 10, 2024
An initial track parameter covariance is necessary to avoid double counting and to estimate the covariance purely from the measurements.

Pulled out of acts-project#2086 to see the individual effects.

I added this to the ODD and ITk full chain and our physmon

bocked by
- acts-project#2829
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes Performance Component - Examples Affects the Examples module Infrastructure Changes to build tools, continous integration, ...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants