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: allow custom precision for volume grid building #2609

Merged
merged 12 commits into from
Nov 9, 2023

Conversation

asalzburger
Copy link
Contributor

This PR allows to set some numerical precision for the volume grid binning, which removes some odd cases when writing out to Json and reading into Detray.

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

Fails unit test at the moment.

Copy link

codecov bot commented Nov 2, 2023

Codecov Report

Merging #2609 (7f603ee) into main (9f7767c) will increase coverage by 0.00%.
The diff coverage is 27.27%.

❗ Current head 7f603ee differs from pull request most recent head 26e8b44. Consider uploading reports for the commit 26e8b44 to get more accurate results

@@           Coverage Diff           @@
##             main    #2609   +/-   ##
=======================================
  Coverage   49.60%   49.60%           
=======================================
  Files         473      473           
  Lines       26828    26833    +5     
  Branches    12355    12357    +2     
=======================================
+ Hits        13308    13311    +3     
  Misses       4753     4753           
- Partials     8767     8769    +2     
Files Coverage Δ
...Acts/Detector/detail/CylindricalDetectorHelper.hpp 59.52% <27.27%> (+0.06%) ⬆️

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@asalzburger
Copy link
Contributor Author

Hey @niermann999 - this is a PR that allows to catch for numerical instabilities in the volume grid.

@asalzburger
Copy link
Contributor Author

Ping @niermann999

niermann999
niermann999 previously approved these changes Nov 9, 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.

Sorry, this had slipped my mind. What issue did it fix?

Co-authored-by: Joana Niermann <53186085+niermann999@users.noreply.github.com>
@acts-policybot acts-policybot bot dismissed niermann999’s stale review November 9, 2023 15:05

Invalidated by push of 26e8b44

@asalzburger
Copy link
Contributor Author

Sorry, this had slipped my mind. What issue did it fix?

It makes rounded values for common boundaries and hence avoid that the json writer invents artificial boundary values with epsilon apart.

@kodiakhq kodiakhq bot merged commit e8ff45a into acts-project:main Nov 9, 2023
54 checks passed
@github-actions github-actions bot removed the automerge label Nov 9, 2023
@paulgessinger paulgessinger modified the milestones: next, v31.0.0 Nov 15, 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.

4 participants