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: Add surface finder container to detector #283

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

niermann999
Copy link
Contributor

@niermann999 niermann999 commented Jul 21, 2022

Add the tuple array container to the detector and puts the toy detector grids inside (Only works on host so far). Apart from that, a brute force surface finder is also added. All can be accessed with custom type IDs, the same way as it is implemented for the masks. The surface finders can be called from the navigator using a neighborhood_kernel, but for now they always return the complete volume range, until the zone function of the grid is properly adapted. The grids are built in the toy geometry in dedicated functions and the detector now uses the custom bin_association to fill surfaces into grids. For now, the grid building in the csv_io.hpp is commented out until properly implemented in a subsequent PR.

Further small changes:

  • the previous surface_finder class is no longer used in favour of the tuple_array_container. All typedefs are done by the type_registry
  • the type ID enums are no longer implicitly convertible to an index for type safety, but can be converted, if necessary
  • Where possible, all code that contains 'unrolling' has been put into functors and is called in the tuple_container.
  • the volume index is now fixed to dindex, since this is unlikely to change
  • the surface finder link has been removed from the masks, since it is now part of the volume class
  • Some minor refactoring (e.g. the edge types are renamed to volume_link for clarity)
  • more elaborate comments on some geometry classes and their template arguments

Edit: For some reason the usage of the detector vecmem resource broke in the navigator jagged vector buffer, so it gets an external host accessible resource now...

@niermann999 niermann999 marked this pull request as ready for review July 26, 2022 09:32
@niermann999 niermann999 force-pushed the feat-make-accel-callable branch from 00f057c to 2c17339 Compare July 26, 2022 09:39
@niermann999 niermann999 requested a review from beomki-yeo July 27, 2022 12:37
@niermann999
Copy link
Contributor Author

The device side code does not yet work, but could you have a first look, please @beomki-yeo ?

@niermann999
Copy link
Contributor Author

The device side code does not yet work, but could you have a first look, please @beomki-yeo ?

Ok, nvm. I need to roll a few things back. I am going to convert it into a draft again

@niermann999 niermann999 marked this pull request as draft July 27, 2022 13:31
@niermann999
Copy link
Contributor Author

Ok, I will rebase and comment the cuda integration in the detector until we have a container that runs on device. Otherwise, I think this can stay the way it is now

@niermann999 niermann999 marked this pull request as ready for review August 12, 2022 19:10
@asalzburger
Copy link
Contributor

There's still some build failures it seems..

@beomki-yeo
Copy link
Collaborator

Is this PR going to be rewritten after #303 ?

@niermann999
Copy link
Contributor Author

Is this PR going to be rewritten after #303 ?

I will try to rebase it. But #303 is mostly orthogonal to the rest of the code base, so it should not cause too much trouble in terms of rebasing

@niermann999 niermann999 force-pushed the feat-make-accel-callable branch 2 times, most recently from 29af60e to 3d11721 Compare October 6, 2022 23:07
detector grids inside (Only works on host so far). Apart from that,
a brute force surface finder is also added. All can be accessed with
custom type IDs, the same way as it is implemented for the masks.
The surface finders can be called from the navigator using a
neighborhood_kernel, but for now they always return the complete
volume range, until the zone function of the grid is properly adapted.
The grids are built in the toy geometry in dedicated functions and
the detector now uses the custom bin_association to fill surfaces
into grids. For now, the grid building is commented out until we have
a working implementation of a grid container in a subsequent PR.

Further small changes:

 - The previous surface_finder class is no longer used in favour of
   the tuple_array_container. All typedefs are done by the
   type_registry.
 - The type ID enums are no longer implicitly convertible to an index
   for type safety, but can be converted, if necessary.
 - Where possible, all code that contains 'unrolling' has been put
   into functors and is called in the tuple_container.
 - The volume index is now fixed to dindex, since this is unlikely to
   change.
 - The surface finder link has been removed from the masks, since it
   is now part of the volume class.
 - Some minor refactoring (e.g. the edge types are renamed to
   volume_link for clarity).
 - More elaborate comments on some geometry classes and their
   template arguments.
 - For some reason, the usage of the detector memory resource in the
   navigator buffer broke, so they get an external resource now.
@niermann999 niermann999 force-pushed the feat-make-accel-callable branch from 144268e to e856af4 Compare October 7, 2022 09:46
@niermann999 niermann999 requested a review from beomki-yeo October 7, 2022 11:37
@niermann999 niermann999 added the enhancement New feature or request label Oct 8, 2022
@beomki-yeo beomki-yeo merged commit e856af4 into acts-project:main Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants