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: adding features for traccc data production #2845

Merged
merged 14 commits into from
Dec 20, 2023

Conversation

asalzburger
Copy link
Contributor

@asalzburger asalzburger commented Dec 19, 2023

This PR adds some necessary fixes for producing traccc data, particularly the possibility to set and assign custom GeometryID values to the new Acts::Detector.

It also contains a missing fix for unit conversion DD4hep/ACTS.

@asalzburger asalzburger added this to the next milestone Dec 19, 2023
@github-actions github-actions bot added Component - Core Affects the Core module Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins labels Dec 19, 2023
@asalzburger
Copy link
Contributor Author

Hey @niermann999 - this is the necessary code to retrieve / set the identifiers on the ACTS side to a Acts::Detector - it does not yet contain the geometry fixes which should come shortly after.

@github-actions github-actions bot added the Component - Documentation Affects the documentation label Dec 19, 2023
Copy link
Contributor

@niermann999 niermann999 left a comment

Choose a reason for hiding this comment

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

Just nitpicky comments, apply as you like

Core/include/Acts/Detector/GeometryIdGenerator.hpp Outdated Show resolved Hide resolved
Core/include/Acts/Detector/GeometryIdGenerator.hpp Outdated Show resolved Hide resolved
Core/include/Acts/Detector/GeometryIdMapper.hpp Outdated Show resolved Hide resolved
Core/include/Acts/Detector/GeometryIdMapper.hpp Outdated Show resolved Hide resolved
Plugins/DD4hep/src/DD4hepDetectorStructure.cpp Outdated Show resolved Hide resolved
Core/include/Acts/Detector/GeometryIdMapper.hpp Outdated Show resolved Hide resolved
Tests/UnitTests/Core/Detector/GeometryIdGeneratorTests.cpp Outdated Show resolved Hide resolved
Core/include/Acts/Detector/GeometryIdGenerator.hpp Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 19, 2023

Codecov Report

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

Comparison is base (288471d) 48.86% compared to head (558c037) 48.87%.

Files Patch % Lines
Core/include/Acts/Detector/GeometryIdGenerator.hpp 54.83% 8 Missing and 6 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2845   +/-   ##
=======================================
  Coverage   48.86%   48.87%           
=======================================
  Files         485      485           
  Lines       28175    28206   +31     
  Branches    13280    13288    +8     
=======================================
+ Hits        13769    13786   +17     
- Misses       4821     4829    +8     
- Partials     9585     9591    +6     

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

niermann999
niermann999 previously approved these changes Dec 19, 2023
@asalzburger
Copy link
Contributor Author

@niermann999 - I had a python formatting issue, please re-approve.

niermann999
niermann999 previously approved these changes Dec 19, 2023
@acts-policybot acts-policybot bot dismissed niermann999’s stale review December 20, 2023 08:16

Invalidated by push of 087f5bf

@asalzburger
Copy link
Contributor Author

Hey @niermann999 - had python formatting problem and a clang-tidy complaint, so please a hopefully final re-approval

Examples/Scripts/Python/geometry.py Show resolved Hide resolved
Examples/Scripts/Python/geometry.py Show resolved Hide resolved
@asalzburger asalzburger merged commit c224fe3 into acts-project:main Dec 20, 2023
52 checks passed
@asalzburger asalzburger deleted the feat-traccc-fixes branch December 20, 2023 10:58
@acts-project-service
Copy link
Collaborator

acts-project-service commented Dec 20, 2023

✅ Athena integration test results [c224fe3]

✅ 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_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

@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Dec 20, 2023
@paulgessinger paulgessinger modified the milestones: next, v32.0.0 Jan 19, 2024
@paulgessinger paulgessinger removed the Breaks Athena build This PR breaks the Athena build label Jan 23, 2024
LaraCalic pushed a commit to LaraCalic/acts that referenced this pull request Feb 10, 2024
This PR adds some necessary fixes for producing traccc data,
particularly the possibility to set and assign custom GeometryID values
to the new `Acts::Detector`.

It also contains a missing fix for unit conversion DD4hep/ACTS.
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 Component - Documentation Affects the documentation Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants