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: update support surface building #2879

Merged

Conversation

asalzburger
Copy link
Contributor

This PR:

  • changes how the definition of ProtoSupport is done
  • renames the SupportHelper to SupportSurfacesHelper
  • adds rectangular support shapes to SupportSurfacesHelper and LayerStructureBuilder
  • extends the Unit tests

@asalzburger asalzburger added this to the next milestone Jan 18, 2024
@github-actions github-actions bot added the Component - Core Affects the Core module label Jan 18, 2024
Copy link

codecov bot commented Jan 18, 2024

Codecov Report

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

Comparison is base (c958c74) 48.98% compared to head (59cfb15) 48.90%.

Files Patch % Lines
Core/src/Detector/detail/SupportSurfacesHelper.cpp 41.61% 20 Missing and 81 partials ⚠️
Core/src/Detector/LayerStructureBuilder.cpp 20.00% 15 Missing and 13 partials ⚠️
Core/include/Acts/Detector/ProtoBinning.hpp 40.00% 1 Missing and 2 partials ⚠️
Core/src/Detector/CylindricalContainerBuilder.cpp 33.33% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2879      +/-   ##
==========================================
- Coverage   48.98%   48.90%   -0.08%     
==========================================
  Files         494      494              
  Lines       28742    28822      +80     
  Branches    13606    13668      +62     
==========================================
+ Hits        14078    14095      +17     
- Misses       4854     4874      +20     
- Partials     9810     9853      +43     

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

@asalzburger
Copy link
Contributor Author

Hi @Corentin-Allaire - I requested this one for you. This is the way how to automatically build support (i.e. passive) material surfaces in the new geometry model, you can tell via a ProtoSupport class where to put it, or provide it directly.

I will have a follow-up PR which shows this on the Open Data Detector.

Copy link
Contributor

@Corentin-Allaire Corentin-Allaire left a comment

Choose a reason for hiding this comment

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

Look good to me ! I have noted some small mistake in the comments but except from that it looks good

Core/src/Detector/detail/SupportSurfacesHelper.cpp Outdated Show resolved Hide resolved
asalzburger and others added 4 commits January 24, 2024 17:48
Co-authored-by: Corentin ALLAIRE <62873125+Corentin-Allaire@users.noreply.github.com>
Co-authored-by: Corentin ALLAIRE <62873125+Corentin-Allaire@users.noreply.github.com>
@asalzburger
Copy link
Contributor Author

linux-nodep has still the timeout to the server ... overriding the blocking CI here.

@asalzburger asalzburger merged commit b0432e5 into acts-project:main Jan 25, 2024
50 of 52 checks passed
@asalzburger asalzburger deleted the feat-update-support-surface-building branch January 25, 2024 05:09
@acts-project-service
Copy link
Collaborator

✅ Athena integration test results [b0432e5]

✅ 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

kodiakhq bot pushed a commit that referenced this pull request Jan 29, 2024
This PR sits on top of #2879 and uses the new `ProtoBinning`, `ProtoMaterial` and `ProtoSupport` schema within the DD4hep context.

It allows to assign `ProtoBinning` to translated support surfaces, but also to define support via DD4hep.

This is the first part of this, a dedicated filling of `acts_proto_support` will follow in a second PR.
@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
This PR: 

- changes how the definition of `ProtoSupport` is done
- renames the `SupportHelper` to `SupportSurfacesHelper`
- adds rectangular support shapes to `SupportSurfacesHelper` and
`LayerStructureBuilder`
- extends the Unit tests

---------

Co-authored-by: Corentin ALLAIRE <62873125+Corentin-Allaire@users.noreply.github.com>
LaraCalic pushed a commit to LaraCalic/acts that referenced this pull request Feb 10, 2024
This PR sits on top of acts-project#2879 and uses the new `ProtoBinning`, `ProtoMaterial` and `ProtoSupport` schema within the DD4hep context.

It allows to assign `ProtoBinning` to translated support surfaces, but also to define support via DD4hep.

This is the first part of this, a dedicated filling of `acts_proto_support` will follow in a second PR.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants