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: introduce blueprint #2550

Merged
merged 25 commits into from
Oct 20, 2023

Conversation

asalzburger
Copy link
Contributor

This PR is the next attempt to introduce an instruction tree for detector building, this is a follow-up of discussion with @paulgessinger and is meant as a replacement / alternative solution to #2537. For this PR - instead of proxies - the simple construction tree is created, momentarily for cylindrical detectors.

This allows the building process to be displayed as a graph:

Screenshot 2023-10-17 at 13 01 09

A Helper (currently for cylindrical detectors only

It then allows also for sorting the nodes in "R" and "Z":

  • before sorting:
    Screenshot 2023-10-17 at 13 46 18

  • after sorting:
    Screenshot 2023-10-17 at 13 46 31

It also allows to insert gap nodes:

  • before inserting:
    Screenshot 2023-10-17 at 15 36 06

  • after inserting:
    Screenshot 2023-10-17 at 15 47 20

In a follow-up PR it will be shown how to work with the CylindricalContainerBuilder schema and eventually a DD4hep test.

@asalzburger asalzburger added this to the next milestone Oct 17, 2023
@github-actions github-actions bot added the Component - Core Affects the Core module label Oct 17, 2023
@paulgessinger
Copy link
Member

paulgessinger commented Oct 17, 2023

Haven't looked at the code yet, but the visualization is hot stuff! 🔥

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #2550 (d293941) into main (fa932cf) will increase coverage by 0.04%.
Report is 1 commits behind head on main.
The diff coverage is 58.06%.

❗ Current head d293941 differs from pull request most recent head 955d617. Consider uploading reports for the commit 955d617 to get more accurate results

@@            Coverage Diff             @@
##             main    #2550      +/-   ##
==========================================
+ Coverage   49.76%   49.81%   +0.04%     
==========================================
  Files         466      468       +2     
  Lines       26353    26508     +155     
  Branches    12100    12182      +82     
==========================================
+ Hits        13114    13204      +90     
- Misses       4632     4633       +1     
- Partials     8607     8671      +64     
Files Coverage Δ
Core/include/Acts/Detector/Blueprint.hpp 70.58% <70.58%> (ø)
Core/src/Detector/detail/BlueprintHelper.cpp 51.92% <51.92%> (ø)

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

niermann999
niermann999 previously approved these changes Oct 18, 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 minor comments

Core/include/Acts/Detector/Blueprint.hpp Outdated Show resolved Hide resolved
Core/include/Acts/Detector/Blueprint.hpp Outdated Show resolved Hide resolved
Core/src/Detector/detail/BlueprintHelper.cpp Outdated Show resolved Hide resolved
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 like it!

Core/src/Detector/detail/BlueprintHelper.cpp Outdated Show resolved Hide resolved
@asalzburger
Copy link
Contributor Author

Hi @niermann999 @paulgessinger - I think I addressed all your comments now, please have a look if you're ok with the PR.

@kodiakhq kodiakhq bot merged commit 187b83c into acts-project:main Oct 20, 2023
54 checks passed
andiwand pushed a commit to andiwand/acts that referenced this pull request Oct 20, 2023
This PR is the next attempt to introduce an instruction tree for detector building, this is a follow-up of discussion with @paulgessinger and is meant as a replacement / alternative solution to acts-project#2537. For this PR - instead of proxies - the simple construction tree is created, momentarily for cylindrical detectors.

This allows the building process to be displayed as a graph:

![Screenshot 2023-10-17 at 13 01 09](https://github.com/acts-project/acts/assets/26623879/8eb39ccb-2f77-4af0-9f89-cfb9d62ebd74)

 **A Helper (currently for cylindrical detectors only** 

It then allows also for sorting the nodes in "R" and "Z":
- before sorting:
![Screenshot 2023-10-17 at 13 46 18](https://github.com/acts-project/acts/assets/26623879/0e4a46d4-8a97-4801-a753-176e757e946a)

- after sorting:
 ![Screenshot 2023-10-17 at 13 46 31](https://github.com/acts-project/acts/assets/26623879/ba227db4-01a4-4490-8f44-e4cd4dcdd32a)

It also allows to insert gap nodes:
- before inserting: 
![Screenshot 2023-10-17 at 15 36 06](https://github.com/acts-project/acts/assets/26623879/5812bb57-1922-42b3-95e8-1bba841c8363)

- after inserting: 
![Screenshot 2023-10-17 at 15 47 20](https://github.com/acts-project/acts/assets/26623879/0aaed0d6-4d84-42eb-a2aa-5e31765d7dde)

In a follow-up PR  it will be shown how to work with the `CylindricalContainerBuilder` schema and eventually a DD4hep test.
kodiakhq bot pushed a commit that referenced this pull request Oct 25, 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`:

<img width="750" alt="Screenshot 2023-10-20 at 16 55 51" src="https://github.com/acts-project/acts/assets/26623879/5ee546db-9581-47be-82fe-32212a08cb9b">
@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