feat: Bin adjustment for rectangluar bounds #705
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
February 9, 2021 11:09
…ment.hpp and modified BinUtility adjustBinUtility(const BinUtility& bu, const Surface& surface) accordingly. This is to make material mapping on renctangle layers possible. Modified the envelope from 1 mm to 0 mm in the TGeoLayerBuilder.hpp (otherwise I get a Radialbounds error when executing ActsExampleGeometryTGeo).
…ty& bu, const Surface& surface) for rectangular bounds.
Codecov Report
@@ Coverage Diff @@
## master #705 +/- ##
==========================================
- Coverage 49.02% 49.01% -0.01%
==========================================
Files 325 325
Lines 16547 16574 +27
Branches 7727 7744 +17
==========================================
+ Hits 8112 8124 +12
- Misses 3012 3019 +7
- Partials 5423 5431 +8
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks for the very nice PR, have have a couple of minor comments.
Two more thing :
- You should run the CI/check_format script in you acts directory then recommit the change, this is needed to pass the format CI check. For that you might need to download clang-format
- It would be nice if you could write a quick unitTest to test the BinAdjustment. Basically just add a test to Tests/UnitTests/Core/Utilities/BinAdjustmentTests.cpp (please ask if you need help)
added 4 commits
February 9, 2021 15:49
also split the if that checks for a plane surface and rectangular bounds in two.
… into rectangular-binning
asalzburger
approved these changes
Feb 9, 2021
asalzburger
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added a function adjustBinUtility() for rectangle bounds to BinAdjustment.hpp and modified BinUtility adjustBinUtility(const BinUtility& bu, const Surface& surface) accordingly. This is to make material mapping on rectangular layers possible.
I also had to modify the envelope from 1 mm to 0 mm in the TGeoLayerBuilder.hpp (otherwise I get a Radialbounds error when executing ActsExampleGeometryTGeo) (This could however be specific to my geometry).