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

refactor: Refactor Examples to use tracks instead of trajectories #2560

Merged
merged 37 commits into from
Oct 28, 2023
Merged

refactor: Refactor Examples to use tracks instead of trajectories #2560

merged 37 commits into from
Oct 28, 2023

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Oct 20, 2023 •
edited
Loading

Currently we mix the usage of track and trajectory containers. In this PR I replace all the trajectory inputs to our writers in favor of track containers.

blocked by

@andiwand andiwand added this to the next milestone Oct 20, 2023
@github-actions github-actions bot added Infrastructure Changes to build tools, continous integration, ... Component - Examples Affects the Examples module Component - Documentation Affects the documentation Vertexing labels Oct 20, 2023
@codecov
Copy link

codecov bot commented Oct 20, 2023 •
edited
Loading

Codecov Report

Merging #2560 (91d0be8) into main (166c57a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2560   +/-   ##
=======================================
  Coverage   49.63%   49.63%           
=======================================
  Files         471      471           
  Lines       26690    26689    -1     
  Branches    12278    12277    -1     
=======================================
  Hits        13247    13247           
  Misses       4748     4748           
+ Partials     8695     8694    -1
Files Coverage Δ
Core/include/Acts/EventData/TrackProxy.hpp 74.60% <ø> (ø)
Core/include/Acts/TrackFinding/TrackSelector.hpp 77.23% <100.00%> (-0.64%) ⬇️
...re/include/Acts/TrackFitting/GaussianSumFitter.hpp 44.00% <ø> (+0.57%) ⬆️

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

@github-actions
Copy link

github-actions bot commented Oct 20, 2023 •
edited
Loading

📊: Physics performance monitoring for 91d0be8

Full contents

physmon summary

paulgessinger
paulgessinger reviewed Oct 20, 2023
View reviewed changes
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.

Still draft, I know, but a handful of comments:

Examples/Io/Performance/ActsExamples/Io/Performance/CKFPerformanceWriter.cpp Outdated Show resolved Hide resolved
Examples/Io/Root/src/RootTrackSummaryWriter.cpp Outdated Show resolved Hide resolved
Examples/Io/Root/src/RootTrackStatesWriter.cpp Outdated Show resolved Hide resolved
Examples/Python/python/acts/examples/reconstruction.py Outdated Show resolved Hide resolved
@andiwand
Copy link
Contributor Author

thanks for the early feedback @paulgessinger !

@andiwand andiwand force-pushed the refactor-examples-use-tracks-instead-of-trajectories branch from f44043f to ba1a208 Compare October 20, 2023 15:33
@github-actions github-actions bot removed Component - Core Affects the Core module Changes Performance labels Oct 20, 2023
@andiwand andiwand force-pushed the refactor-examples-use-tracks-instead-of-trajectories branch from ba1a208 to 4ac193d Compare October 20, 2023 15:45
@github-actions github-actions bot added Component - Core Affects the Core module Event Data Model labels Oct 21, 2023
@andiwand andiwand force-pushed the refactor-examples-use-tracks-instead-of-trajectories branch from 138d06c to 622dd28 Compare October 21, 2023 08:46
benjaminhuth
benjaminhuth previously approved these changes Oct 26, 2023
View reviewed changes
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.

Looks good! let's hope the best for the CI

@acts-policybot acts-policybot bot dismissed benjaminhuth’s stale review October 26, 2023 12:00

Invalidated by push of c8309e8

paulgessinger
paulgessinger previously approved these changes Oct 27, 2023
View reviewed changes
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.

Generally, I'm ok to go ahead with this. Here's two more comments that you can consider.

Examples/Algorithms/Vertexing/src/IterativeVertexFinderAlgorithm.cpp Show resolved Hide resolved
Core/include/Acts/EventData/TrackProxy.hpp Outdated Show resolved Hide resolved
@andiwand
Copy link
Contributor Author

tried to improve this @paulgessinger

paulgessinger
paulgessinger previously approved these changes Oct 28, 2023
View reviewed changes
@paulgessinger
Copy link
Member

I think this picked up a conflict from #2581 unfortunately.

@kodiakhq kodiakhq bot merged commit 0e44bd2 into acts-project:main Oct 28, 2023
55 checks passed
@andiwand andiwand deleted the refactor-examples-use-tracks-instead-of-trajectories branch October 28, 2023 11:20
@acts-project-service
Copy link
Collaborator

acts-project-service commented Oct 28, 2023 •
edited
Loading

✅ Athena integration test results [0e44bd2]

✅ All tests successful

status job
🟢 run_workflow_tests_run2_data
🟢 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/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/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_run3_mc
🟢 run_workflow_tests_run3_data

@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Oct 28, 2023
@paulgessinger paulgessinger modified the milestones: next, v31.0.0 Nov 6, 2023
@paulgessinger paulgessinger removed the Fails Athena tests This PR causes a failure in the Athena tests label Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjaminhuth benjaminhuth benjaminhuth approved these changes

@paulgessinger paulgessinger paulgessinger left review comments

Assignees
No one assigned
Labels
Changes Performance Component - Core Affects the Core module Component - Documentation Affects the documentation Component - Examples Affects the Examples module Event Data Model Infrastructure Changes to build tools, continous integration, ... Track Finding Track Fitting Vertexing
Projects
None yet
Milestone
v31.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@andiwand @paulgessinger @acts-project-service @benjaminhuth