Skip to content

Tackling some sepfir2d improvements #43

@rgommers

Description

@rgommers

From scipy#14367 (comment):

There are a couple of issues (2 open, 2 closed) about sepfir2d, so it does get usage. Now that we've looked into this and found some of the problems, it makes sense to fix them. This could turn into a bit of a project though, so let's not try to do it all in this PR. @Smit-create I suggest the following:

  • This PR: incorporate the fix the outptr -> out in memmove (see review comment above), and then we'll merge this.
  • Follow-up PRs (all separately, or at least the templating one - to keep diffs readable):
    • deduplicating the four files (C_bspline_util.c, D_bspline_util.c, S_bspline_util.c, Z_bspline_util.c). This can be done with a Tempita template (see scipy/signal/correlate_nd.c.in for an example). At that point bspline_util.c also doesn't need to be a separate file, the one function in it can be included in the same templated C file.
    • add a test for strided input arrays, and handle strides correctly (@peterbell10's comment above)
    • add a test for BUG: signal.sepfir2d: fails with complex input on Windows scipy/scipy#13643 and remove the __GNUC__ guards. I think this requires modernizing the code (__complex__ usage is odd, it's only done in a few signal functions)

This code is pretty arcane, it should be possible to squash quite a few bugs in it here.

EDIT: final thing could be to improve the algorithm to also handle even hrow, hcol, see scipygh-12691.

Next steps

Let's start with the easier topics here:

  • deduplicate the four files to get only a single file
  • remove the __GNUC__ guards. Instead of using __complex__ float, this could use numpy types like npy_cfloat.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions