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: use blueprint to generate cylindrical detector #2557

Merged
merged 27 commits into from
Oct 25, 2023

Conversation

asalzburger
Copy link
Contributor

@asalzburger asalzburger commented Oct 19, 2023

This PR follows #2550 and introduces an automated conversion from blueprint to a cylindrical detector builder.

  • it adds the auto-translation of blueprint to cylindrical container builder
  • plus unit test (yes, it's boiler plate blueprint building, but the code for detector building is now really concise)
  • it adds the graphical representation of root volume finder builder and geo id generator:
Screenshot 2023-10-20 at 16 55 51

@asalzburger asalzburger added this to the next milestone Oct 19, 2023
@github-actions github-actions bot added the Component - Core Affects the Core module label Oct 19, 2023
@asalzburger asalzburger changed the title Feat cylindrical blueprint feat: use blueprint to generate cylindrical detector Oct 19, 2023
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #2557 (64fd78a) into main (630fc20) will decrease coverage by 0.02%.
Report is 2 commits behind head on main.
The diff coverage is 37.03%.

❗ Current head 64fd78a differs from pull request most recent head be3e731. Consider uploading reports for the commit be3e731 to get more accurate results

@@            Coverage Diff             @@
##             main    #2557      +/-   ##
==========================================
- Coverage   49.64%   49.63%   -0.02%     
==========================================
  Files         471      471              
  Lines       26632    26679      +47     
  Branches    12237    12270      +33     
==========================================
+ Hits        13222    13242      +20     
- Misses       4742     4743       +1     
- Partials     8668     8694      +26     
Files Coverage Δ
...lude/Acts/Detector/CylindricalContainerBuilder.hpp 100.00% <ø> (ø)
...re/include/Acts/Detector/DetectorVolumeBuilder.hpp 100.00% <ø> (ø)
Core/src/Detector/DetectorVolumeBuilder.cpp 45.00% <0.00%> (ø)
Core/include/Acts/Detector/Blueprint.hpp 68.85% <60.00%> (-1.74%) ⬇️
Core/src/Detector/detail/BlueprintHelper.cpp 52.83% <33.33%> (+0.90%) ⬆️
Core/src/Detector/VolumeStructureBuilder.cpp 29.35% <0.00%> (-2.38%) ⬇️
Core/src/Detector/CylindricalContainerBuilder.cpp 49.07% <40.00%> (-2.21%) ⬇️

... and 1 file with indirect coverage changes

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

@asalzburger
Copy link
Contributor Author

Hoi @paulgessinger - I addressed all your comments, I suppose, do you think this could be merged?

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.

Looks good to me

@paulgessinger
Copy link
Member

paulgessinger commented Oct 24, 2023

If you don't mind I'll have a quick look in a minute.

EDIT: Thank! Let's go!

@asalzburger
Copy link
Contributor Author

Re-triggered pipelines due to CI failures.

@kodiakhq kodiakhq bot merged commit 33e36ea into acts-project:main Oct 25, 2023
53 checks passed
@paulgessinger paulgessinger modified the milestones: next, v30.3.0 Oct 25, 2023
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.

None yet

4 participants