-
Notifications
You must be signed in to change notification settings - Fork 60
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
Fix ImportError from vendored code #252
Fix ImportError from vendored code #252
Conversation
Thanks Greg! 😄 Instead of vendoring what if we do a Agree in the medium term would be good to contribute upstream to avoid needing these internal bits. |
Do you mean modifying try:
import cupyx.scipy.ndimage
if hasattr(cupyx.scipy.ndimage, '_filters'):
# CuPy 11.x
from cupyx.scipy.ndimage._filters import _get_correlate_kernel
else:
from cupyx.scipy.ndimage.filters import _get_correlate_kernel
except ImportError:
import cupy
from cucim.skimage._vendored import _ndimage_filters_core as _filters_core
@cupy.memoize(for_each_device=True)
def _get_correlate_kernel(mode, w_shape, int_type, offsets, cval):
return _filters_core._generate_nd_kernel(
'correlate',
'W sum = (W)0;',
'sum += cast<W>({value}) * wval;',
'y = cast<Y>(sum);',
mode, w_shape, int_type, offsets, cval, ctype='W') That would let us potentially benefit from any upstream improvements or fixes to |
Alright let's go with this as-is, but let's work on upstreaming the rest of what is needed for 22.06. Could you please file an issue to track this? |
@gpucibot merge |
closes #251 by avoiding imports from private CuPy code
The new files here are just subsets of files present in CuPy that are not part of its public API. Only a few functions use functionality from this
_vendored
folder.The whole folder is mainly here in order to have a version of
cupyx.scipy.signal.fftconvolve
with a modifiedchoose_conv_mode
. We should eventually just update the convolution selection code in CuPy so that we can remove these files.I think
fftconvolve
itself was not present at all in CuPy 9.x, but even in the current development branch, the convolution choice function is still restricted to 1D