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

Angle-aware filtering with optional GPU acceleration #525

Merged
merged 22 commits into from
Feb 23, 2022

Conversation

CHrlS98
Copy link
Contributor

@CHrlS98 CHrlS98 commented Nov 18, 2021

Hey!
New script for computing angle-aware bilateral filtering (my ISMRM 2022 abstract). The algorithm can be applied sequentially, across multiple CPU threads or on the GPU (given that pyopencl is installed).

I didn't add pyopencl to the requirements to avoid introducing an additional dependency, but maybe it is something we should consider at some point, as GPU acceleration greatly increases compute time; on fODF from a full brain (HCP with Tractoflow), it takes 12 minutes versus 90 minutes to process using default parameters.

The PR also contains changes to scil_execute_asymmetric_filtering.py and its associated modules to avoid confusion. Also, for both methods, it is now possible to output the asymmetric and the symmetric signal during the same execution.

I put the OpenCL kernel file directly in the denoise module with the python files. Let me know how you feel about this.

I'll also add tests really soon, for the sequential case at least. You can test it with the FiberCup, it runs pretty fast.

To do:

  • Add tests;
  • Documentation for opencl_utils.py.

Thank you.

@scil-admin
Copy link

@scil-admin
Copy link

Build passed ! Good Job 🍻 !

@scil-admin
Copy link

Build passed ! Good Job 🍻 !

@CHrlS98 CHrlS98 changed the title Angle-aware filtering with optional GPU acceleration [WIP] Angle-aware filtering with optional GPU acceleration Nov 19, 2021
@CHrlS98 CHrlS98 changed the title [WIP] Angle-aware filtering with optional GPU acceleration Angle-aware filtering with optional GPU acceleration Nov 19, 2021
@scil-admin
Copy link

Build passed ! Good Job 🍻 !

Copy link
Contributor

@arnaudbore arnaudbore left a comment

Choose a reason for hiding this comment

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

A couple of remaining docstring missing otherwise LGTM.

return out_sf


def correlate_spatial(image_u, h_filter, sigma_range):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add some docstring when needed

@arnaudbore arnaudbore requested a review from mdesco January 19, 2022 15:44
@scil-admin scil-admin requested a review from arnaudbore January 19, 2022 16:58
@scil-admin
Copy link

Build passed ! Good Job 🍻 !

@scil-admin
Copy link

Build passed ! Good Job 🍻 !

Copy link
Contributor

@mdesco mdesco left a comment

Choose a reason for hiding this comment

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

Testing this PR at the moment on my Linux Desktop.

  1. It completely freezes my display as it runs if I use --use_gpu. Is this expected? Essentially, it would be impossible for me to work on my machine at the same time.
  2. Is there a way that we could run the pip install pyopencl for the user?
  3. What is the htop or top way to see what the GPU is doing? (I'm novel to this)

Otherwise, it seems to work...

@CHrlS98
Copy link
Contributor Author

CHrlS98 commented Jan 20, 2022

@mdesco

  1. It does slow down the PC, particularly for softwares also using the GPU. However, I never experienced any of my PCs completely freezing. I guess it could happen if all of your GPU memory were taken by the process.
  2. It isn't added to the requirements.txt as I didn't want to introduce any additional dependencies. We could imagine having an optional requirements file that the user could manually install with pip install -r optional.txt but since there would only be one dependency inside, it's not that better than simply running pip install pyopencl.
  3. For a NVIDIA graphics card, the command is watch nvidia-smi.

What is the size of the dataset you used for testing? Does the output look okay even though it freezes everything?

@CHrlS98
Copy link
Contributor Author

CHrlS98 commented Feb 10, 2022

@mdesco
It is possible that your PC froze because the wrong GPU was selected for the task (if you have more than one GPU, that is).

Therefore, I added a check for selecting the most suitable GPU based on the available memory. I will need to test on a PC with 2 or more GPUs to be sure it works, though.

@scil-admin
Copy link

Build passed ! Good Job 🍻 !

Copy link
Contributor

@mdesco mdesco left a comment

Choose a reason for hiding this comment

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

All good! Good job @CHrlS98

@arnaudbore arnaudbore merged commit 5e7467b into scilus:master Feb 23, 2022
@CHrlS98 CHrlS98 deleted the angle-aware-filter branch February 23, 2022 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants