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: GX2F: Finish material-less fit by resetting the track container #2739

Merged
merged 4 commits into from
Dec 7, 2023

Conversation

AJPfleger
Copy link
Contributor

@AJPfleger AJPfleger commented Nov 28, 2023

It is probably not the most optimal solution but it makes the GX2F (Global Chi Square Fitter) work. Instead of revisiting and updating track states during each iteration, we now flush the track container after each iteration and add them from scratch.

Regarding the performance: The track container works similar to a vector, so by clearing it the space for n track states remains reserved, which makes this approach less bad than one might suspect.

This finally gives us also nice phi-, theta-, and q/p-pulls with the Generic detector. We can also see that we most of the time only need 3-4 updates for convergence.

Generic Detector

MomentumConfig(100.0 * u.GeV, 100.0 * u.GeV, transverse=True),
EtaConfig(-3.0, 3.0),
PhiConfig(0.00 * u.degree, 360.00 * u.degree),
nUpdateMax=17,
zeroField=False,
relChi2changeCutOff=1e-11,

2023-11-28loc0
2023-11-28loc1
2023-11-28phi
2023-11-28theta
2023-11-28qop
2023-11-28nUpdates

@github-actions github-actions bot added Component - Core Affects the Core module Track Fitting labels Nov 28, 2023
Copy link

codecov bot commented Nov 28, 2023

Codecov Report

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

Comparison is base (c65d419) 48.69% compared to head (14ea9ef) 49.56%.

❗ Current head 14ea9ef differs from pull request most recent head dc71e2c. Consider uploading reports for the commit dc71e2c to get more accurate results

Files Patch % Lines
...nclude/Acts/TrackFitting/GlobalChiSquareFitter.hpp 28.57% 1 Missing and 9 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2739      +/-   ##
==========================================
+ Coverage   48.69%   49.56%   +0.86%     
==========================================
  Files         479      474       -5     
  Lines       27894    27059     -835     
  Branches    13173    12502     -671     
==========================================
- Hits        13583    13411     -172     
+ Misses       4765     4748      -17     
+ Partials     9546     8900     -646     

☔ 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 November 28, 2023 15:14
@AJPfleger AJPfleger changed the title fix: GX2F: Finish material-less fit by resetting the track container (fixes all pulls) fix: GX2F: Finish material-less fit by resetting the track container Nov 28, 2023
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.

I would be in favour to get this in, in order to have a first application test outside our ecosystem.

@paulgessinger paulgessinger added this to the next milestone Dec 6, 2023
Copy link
Member

@paulgessinger paulgessinger left a comment

Choose a reason for hiding this comment

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

I think the logging output probably needs an overhaul in any case: multi-line outputs in the logger often look weird anyway. But this is not a blocker for the actual change, so let's go.

@kodiakhq kodiakhq bot merged commit 5cc42d2 into acts-project:main Dec 7, 2023
51 checks passed
@github-actions github-actions bot removed the automerge label Dec 7, 2023
@acts-project-service
Copy link
Collaborator

acts-project-service commented Dec 7, 2023

✅ Athena integration test results

✅ All tests successful

status job report
🟢 run_unit_tests
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsBenchmarkWithSpot.sh 8 100
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsWorkflow.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateAmbiguityResolution.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateResolvedTracks.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateTracks.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateActsCoreSpacePoints.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateActsSpacePoints.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateSeeds.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateOrthogonalSeeds.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateClusters.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsPersistifyEDM.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/test/ActsGSFRefitting.sh
🟢 run_ci_tests: ../athena/AtlasTest/CITest/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_data
🟢 run_workflow_tests_run3_mc
🟢 run_workflow_tests_run3_data
🟢 run_art_test: test_data18_13TeV_1000evt
🟢 run_art_test: test_ttbarPU40_reco

@paulgessinger paulgessinger modified the milestones: next, v31.2.0 Dec 7, 2023
@AJPfleger AJPfleger deleted the gx2f-trackcontainer branch December 19, 2023 14:36
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