Add 2D signal b-splines to cupyx.scipy.signal #7721
Conversation
andfoy
changed the title
| intype = Iin.dtype.char | ||
| hcol = cupy.asarray([1.0, 4.0, 1.0], 'f') / 6.0 | ||
| if intype in ['F', 'D']: | ||
| Iin = Iin.astype('F') |
There was a problem hiding this comment.
This is strange (both here and in scipy) : why does it only work in single precision complex.
Also below, is there a major reason sepfir2d cannot natively support complex inputs?
There was a problem hiding this comment.
As you mention it, this is directly copied from SciPy, no modifications so far. We would need to research more about the sepfir implementation.
My guess is that (from what I've seen), all the implementations in _splines_mod.c.in (Or that template file in SciPy) only define functions for floating point numbers and not for other data types
EDIT: CuPy sepfir2 does support complex dtypes
|
This pull request is now in conflicts. Could you fix it @andfoy? 🙏 |
andfoy
force-pushed
the
add_2d_signal_splines
branch
from
f1b1298 to
467eb1f
Compare
|
/test mini |
| @@ -393,3 406,111 @@ def qspline1d_eval(cj, newx, dx=1.0, x0=0): | |||
| result = cj[indj] * _quadratic(newx - thisj) | |||
| res[cond3] = result | |||
| return res | |||
|
|
|||
|
|
|||
| def cspline2d(signal, lamb=0.0, precision=-1.0): | |||
There was a problem hiding this comment.
Seems that scipy uses lambda for the name of the second argument. Any reason to choose lamb here?
https://docs.scipy.org/doc/scipy/reference/generated/scipy.signal.cspline2d.html
There was a problem hiding this comment.
As lambda is a reserved keyword in Python, then it is not possible to use it. Since SciPy is generating the signature from C, I think it would be possible to declare lambda as a parameter in the exported function. However in the practice, any use of lambda will trigger a SyntaxError. https://github.com/scipy/scipy/blob/897d811c616ec380df4000e117034e0c4cb2815c/scipy/signal/_splinemodule.c#L22
This is a bug in SciPy
| return out | ||
|
|
||
|
|
||
| def qspline2d(signal, lamb=0.0, precision=-1.0): |
|
Some tests seem to have mismatched values. Would you check them? |
|
The culprit is When it was passing: I don't know if this is a new behaviour or a regression caused after #7651 |
|
I can confirm the tests are fixed after #7815 |
|
This pull request is now in conflicts. Could you fix it @andfoy? 🙏 |
andfoy
force-pushed
the
add_2d_signal_splines
branch
from
467eb1f to
42c706a
Compare
|
/test full |
|
Thanks, @andfoy! |
See #7403
Depends on #7715