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: Implement surface lookup from source link #576

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

niermann999
Copy link
Contributor

@niermann999 niermann999 commented Oct 24, 2023

This abstracts the surface lookup from the detector class, so that it can be extended later on without breaking anything. The surface lookup is renamed to just surfaces again. It also strips the source link from the surface descriptor, which will now not be carried around throughout all of the navigation and grid. The source link is still available via the surface lookup.

In order to make easier lookups for the source links, the portal, sensisitve and passive index ranges are added back to the volume descriptor, which also allows to get access to the surface range of a volume via the detector_volume class. Wherever I could find it, I already refactored the surface access through the volume accordingly.

@niermann999 niermann999 force-pushed the feat-new-sf-lookup branch 8 times, most recently from f981d0b to e21c1ec Compare October 28, 2023 22:43
@niermann999 niermann999 force-pushed the feat-new-sf-lookup branch 3 times, most recently from 196b9ab to 9bff1e6 Compare November 21, 2023 18:00
@niermann999 niermann999 force-pushed the feat-new-sf-lookup branch 4 times, most recently from eefb871 to 94ce91a Compare December 19, 2023 16:07
@niermann999 niermann999 marked this pull request as ready for review December 19, 2023 16:28
@niermann999 niermann999 added enhancement New feature or request refactor refactoring the current codes priority: high high priority labels Dec 19, 2023
@niermann999
Copy link
Contributor Author

Most of the changes in the detector are from reordering the methods, so that it becomes clear which ones are up for removal

@niermann999 niermann999 force-pushed the feat-new-sf-lookup branch 9 times, most recently from 2e89e27 to 6a96e9c Compare January 4, 2024 11:30
@niermann999 niermann999 force-pushed the feat-new-sf-lookup branch 9 times, most recently from 80aeb02 to 8559685 Compare January 8, 2024 11:54
@niermann999 niermann999 requested a review from beomki-yeo January 8, 2024 11:54
@niermann999
Copy link
Contributor Author

I need this PR for the grid builder, so that I can use the index offsets in the volume descriptors to switch between global and local indices in the grids

@niermann999
Copy link
Contributor Author

niermann999 commented Jan 8, 2024

With some help from @beomki-yeo , I also hot-fixed another division by zero and added some asserts

… can be extended

later on without breaking anything. The surface lookup is renamed to just surfaces again.
It also strips the source link from the surface descriptor, which will now
not be carried around throughout all of the navigation and grid. The source link
is still available via the surface lookup.

In order to make easier lookups for the source links, the portal, sensisitve
and passive index ranges are added back to the volume descriptor, which also allows
to get access to the surface range of a volume via the detector_volume class.
Wherever I could find it, I already refactored the surface access through the volume accordingly.
Copy link
Collaborator

@beomki-yeo beomki-yeo 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

@niermann999 niermann999 merged commit 7a4cd11 into acts-project:main Jan 11, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority: high high priority refactor refactoring the current codes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants