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

Birmingham Method on GPU #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dwerner95
Copy link
Member

Hey All,

The Birmingham-Method is the oldest algorithm to determine the tracer location from PEPT data. Therefore it does not use state of the art libraries that provide fancy functions as HDBScan or topological functions.
As a result, the Birmingham-Method is quite simple to implement on GPU, especially because the already existing C-code does only need few modifications to be compatible with CUDA. With this pull request I offer a initial running version of the Birmingham-Method implemented in CUDA running via pyCUDA.

Minimal Documentation is provided.

This GPU-Birmingham-Method is implemented as an Reducer instead of a Filter, this means that there is no implementation of fit_sample but a implementation of fit. This results in two things, first of all it is not influenced by the automatic parallelization inside a pipeline, so you can, if you want, create a single pipeline with the GPU-Birmingham-Method first then cluster the points using HDBScan and second, there is no need to stack the result after a run as it technically did not run parallel. Not sure if the implementation is correct, but i leave it for you @anicusan to decide ;-)

The function is tested against the CPU-Birmingham-Method and shows minimal differences, which is to be expected considering the lower precision used on GPUs.

CPU GPU
Time 33.3s 8.0s

The errors calculated between CPU and GPU as the mean difference between each point:

Error Value
Time 0.003 ms
X 3.2e-5 mm
Y 1.8e-4 mm
Z 1.0e-4 mm

This all was just a fun project to me, so I'd be careful using this before someone else did review it properly. I know there is one issue that I can't resolve right now, it has to do with overlaps so only occurs if overlap > 0, but i am sure we can sort that out together.

Have a great holiday!

@dwerner95 dwerner95 added the enhancement New feature or request label Dec 30, 2022
@dwerner95 dwerner95 requested a review from anicusan December 30, 2022 11:17
@anicusan
Copy link
Member

This is faaantastic! The batching system you wrote seems great for future GPU algorithms - perhaps we could generalise that into its own class?

Also, before merging, could you please:

  • Add a few tests into tests/test_tracking.py and run pytest in that directory?
  • Add a documentation entry in docs/source/manual?

# gpu imports
# make imports global. There may be a better solution for this
global cuda, SourceModule, DynamicSourceModule
try:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add these imports at the top and let the code crash upon first use if the user doesn't have pycuda; you first use it when cuda.Device.count() - add the error message as a comment above that line

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.

2 participants