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: Improvements to Cuboid Volume Builder #1166

Merged
merged 16 commits into from
Mar 9, 2022

Conversation

pbutti
Copy link
Contributor

@pbutti pbutti commented Feb 16, 2022

Adds a number of changes to the CuboidVolumeBuilder to make it more robust.

paulgessinger and others added 9 commits November 17, 2021 17:29
Also adds a way to provide an envelope in x separate from the surface
thickness. This is needed for multi-sensor layers. The auto volume
binning should now also take the envelope into account when calculating
the binning.
fix: Passing the created protoLayer to the planeLayer factory
Fixed initialization issue in centroid estimation for multiple surface layer
Added explicit sort of the volumes vector by their center X location
@paulgessinger paulgessinger added this to the next milestone Feb 16, 2022
@paulgessinger
Copy link
Member

Thanks @pbutti! I set the milestone and the CI should run now.

@paulgessinger
Copy link
Member

I'll fix the build errors.

@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #1166 (1ba59e5) into main (40d6e79) will decrease coverage by 0.02%.
The diff coverage is 29.03%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1166      +/-   ##
==========================================
- Coverage   47.83%   47.80%   -0.03%     
==========================================
  Files         360      360              
  Lines       18530    18549      +19     
  Branches     8739     8756      +17     
==========================================
+ Hits         8863     8867       +4     
- Misses       3631     3633       +2     
- Partials     6036     6049      +13     
Impacted Files Coverage Δ
Core/include/Acts/Geometry/CuboidVolumeBuilder.hpp 33.33% <ø> (ø)
Core/src/Geometry/CuboidVolumeBuilder.cpp 35.83% <29.03%> (-2.79%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40d6e79...1ba59e5. Read the comment docs.

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.

The code makes sense to me

Core/src/Geometry/CuboidVolumeBuilder.cpp Outdated Show resolved Hide resolved
Core/src/Geometry/CuboidVolumeBuilder.cpp Show resolved Hide resolved
Core/src/Geometry/CuboidVolumeBuilder.cpp Outdated Show resolved Hide resolved
@pbutti
Copy link
Contributor Author

pbutti commented Mar 1, 2022

Opened a PR to the base branch in Paul's fork. When that is merged, the commits will be picked up.

@paulgessinger paulgessinger added automerge Component - Core Affects the Core module Improvement Changes to an existing feature labels Mar 2, 2022
@paulgessinger paulgessinger merged commit 6b30469 into acts-project:main Mar 9, 2022
@paulgessinger paulgessinger deleted the feat/cubvolbld-ldmx branch March 9, 2022 15:34
@paulgessinger paulgessinger removed this from the next milestone Apr 4, 2022
@paulgessinger paulgessinger added this to the v18.0.0 milestone Apr 4, 2022
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 Improvement Changes to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants