-
Notifications
You must be signed in to change notification settings - Fork 4
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
Port more stripe removal methods from TomoPy to CuPy #40
base: main
Are you sure you want to change the base?
Conversation
Similar to other ports to CuPy, the residual is non-zero between the CuPy version and the CPU TomoPy version, but small. Notably, the result of both implementations when using default values for all parameters produces projection images with noisy artifacts. It is primarily the regions of noise which have the greatest difference in the residual. Currently, the reason for these artifacts is unknown, but one possibility is that this particular stripe removal method is not suited for the test data (ie, possibly no "large" stripes exist in the test data).
Similar comments here as the large stripe removal port in 8af3aa2 regarding the residual between the TomoPy/CPU implementation and the CuPy/GPU implementation. Something else worth noting is that the TomoPy version of this method uses a 2D interpolation function from scipy https://docs.scipy.org/doc/scipy/reference/generated/scipy.interpolate.interp2d.html. The current latest version of CuPy (v11.5.0) does not have an analogue of this function, but the pre-release version v.12.0.0b3 does, it has an n-dimensional interpolation function that serves well as a GPU replacement of the scipy `interp2d()` function https://github.com/cupy/cupy/releases/tag/v12.0.0b3. The required function `interpn()` in the CuPy codebase and its dependent functions https://github.com/cupy/cupy/blob/1a9c91411fe6297ea81c6ad103cd8211134f6446/cupyx/scipy/interpolate/_rgi.py have been copied into the httomolib codebase for the moment, as it was the simplest way to incorporate the required interpolation function.
@dkazanc @namannimmo10 Heads-up, the port of dead stripe removal involves something that people may find a bit dodgy, please read the commit message for e18feaf and let me know if you think an alternative should be done instead! |
This method is simply running different stripe removal methods in sequence: - removal of unresponsive and fluctuating stripes, via `_rs_dead()` - removal of full and partial stripes, via `_rs_sort()` and is doing nothing new. Therefore, the non-zero residual between the TomoPy/CPU and CuPy/GPU implementations of this particular method is due to the non-zero residuals present for the dependent stripe removal methods, rather than some new processing being introduced by `remove_all_stripe_cupy()` which adds to the residual.
@yousefmoazzam I think it is fine to use the ported function for now but we need to make a mark that it should be replaced with the 12.0 rlease of CuPy? |
@dkazanc Yep, that makes sense. I've put code comments in the method https://github.com/DiamondLightSource/httomolib/blob/0f4e060b4510efd42450aa199cf4217475d21896/src/httomolib/prep/stripe.py#L247-L250 and at the top of the copied file https://github.com/DiamondLightSource/httomolib/blob/0f4e060b4510efd42450aa199cf4217475d21896/src/httomolib/prep/_rgi.py#L1-L4 but if there is clearer/more obvious way to mark it then feel free to do so. |
This PR will be providing CuPy ports of the methods mentioned in item 1 from #22.