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.

Sign up for GitHub

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

Jump to bottom

fix: Initial track param covariance inflation #2295

Merged
merged 46 commits into from
Jan 24, 2024
Merged

fix: Initial track param covariance inflation #2295

merged 46 commits into from
Jan 24, 2024

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Jul 11, 2023 •
edited
Loading

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

@andiwand andiwand added this to the next milestone Jul 11, 2023
@github-actions github-actions bot added Infrastructure Changes to build tools, continous integration, ... Component - Examples Affects the Examples module labels Jul 11, 2023
@CarloVarni CarloVarni added the 🚧 WIP Work-in-progress label Jul 11, 2023
@codecov
Copy link

codecov bot commented Jul 11, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (098afd5) 48.98% compared to head (1c077f0) 48.98%.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2295   +/-   ##
=======================================
  Coverage   48.98%   48.98%           
=======================================
  Files         494      494           
  Lines       28742    28742           
  Branches    13606    13606           
=======================================
  Hits        14078    14078           
  Misses       4854     4854           
  Partials     9810     9810

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

@github-actions
Copy link

github-actions bot commented Jul 11, 2023 •
edited
Loading

📊: Physics performance monitoring for 1c077f0

Full contents

physmon summary

@andiwand andiwand mentioned this pull request Jul 11, 2023
Merged
kodiakhq bot pushed a commit that referenced this pull request Jul 24, 2023
@andiwand
fix: Improve full chain pulls by enabling Fatras interactions (#2086)
dbcba62 
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
- #1643

Blocked by
- #2157
- #2239
- #2295
- #2293
- #2294
paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Jul 24, 2023
@andiwand @paulgessinger
fix: Improve full chain pulls by enabling Fatras interactions (acts-p…
9e75691 
…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
@andiwand andiwand marked this pull request as ready for review July 25, 2023 11:39
@github-actions github-actions bot added the Stale label Sep 13, 2023
@github-actions github-actions bot removed the Stale label Sep 27, 2023
@paulgessinger paulgessinger marked this pull request as draft September 29, 2023 08:42
@paulgessinger paulgessinger removed the 🚧 WIP Work-in-progress label Sep 29, 2023
@acts-policybot acts-policybot bot dismissed felix-russo’s stale review December 22, 2023 16:11

Invalidated by push of fb1c7a4

@andiwand
Copy link
Contributor Author

I made this dependent on #2829 to show the combined performance

@andiwand andiwand removed the 🛑 blocked This item is blocked by another item label Jan 18, 2024
@paulgessinger
Copy link
Member

I think the hashes are still different.

@kodiakhq kodiakhq bot merged commit a739299 into acts-project:main Jan 24, 2024
54 checks passed
@acts-project-service
Copy link
Collaborator

✅ Athena integration test results [a739299]

✅ All tests successful

status job report
🟢 run_unit_tests
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsEFTrackFit.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsBenchmarkWithSpot.sh 8 100
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsWorkflow.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsValidateAmbiguityResolution.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsValidateResolvedTracks.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsValidateTracks.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsValidateActsCoreSpacePoints.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsValidateActsSpacePoints.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsValidateSeeds.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsValidateOrthogonalSeeds.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsValidateClusters.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsPersistifyEDM.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsGSFRefitting.sh
🟢 run_ci_tests: ../athena/Tracking/Acts/ActsConfig/test/ActsKfRefitting.sh
🟢 run_ci_tests: python3 ../athena/Tracking/Acts/ActsGeometry/test/ActsExtrapolationAlgTest.py
🟢 run_ci_tests: python3 ../athena/Tracking/Acts/ActsGeometry/test/ActsITkTest.py
🟢 run_workflow_tests_run4_mc
🟢 run_workflow_tests_run2_mc
🟢 run_workflow_tests_run2_data
🟢 run_workflow_tests_run3_mc
🟢 run_workflow_tests_run3_data
🟢 run_art_test: test_data18_13TeV_1000evt
🟢 run_art_test: test_ttbarPU40_reco

@andiwand andiwand deleted the full-chain-cov-inflation branch January 24, 2024 12:59
@paulgessinger paulgessinger modified the milestones: next, v32.1.0 Feb 2, 2024
LaraCalic pushed a commit to LaraCalic/acts that referenced this pull request Feb 10, 2024
@andiwand @LaraCalic
fix: Initial track param covariance inflation (acts-project#2295)
b40c523 
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
@andiwand andiwand mentioned this pull request May 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulgessinger paulgessinger paulgessinger approved these changes

@felix-russo felix-russo felix-russo left review comments

Assignees
No one assigned
Labels
Changes Performance Component - Core Affects the Core module Component - Examples Affects the Examples module Infrastructure Changes to build tools, continous integration, ... Seeding Track Finding
Projects
None yet
Milestone
v32.1.0
Development

Successfully merging this pull request may close these issues.
5 participants
@andiwand @felix-russo @paulgessinger @acts-project-service @CarloVarni