Add pyfftw sdp by NimaSarajpoor · Pull Request #1132 · stumpy-dev/stumpy

NimaSarajpoor · 2026-02-17T05:38:14Z

This is to address PR 3 described in #1118 (comment). Have copied the corresponding notes below:

Add _PYFFTW_SLIDING_DOT_PRODUCT to sdp.py

Add unit tests that demonstrate that _PYFFTW_SLIDING_DOT_PRODUCT matches the results of naive_rolling_window_dot_product

Handle the test.sh check_fftw_pyfftw stuff here too

gitnotebooks · 2026-02-17T05:38:19Z

Review these changes at https://app.gitnotebooks.com/stumpy-dev/stumpy/pull/1132

NimaSarajpoor · 2026-02-17T05:39:28Z

stumpy/sdp.py

+try:  # pragma: no cover
+    import pyfftw
+
+    FFTW_IS_AVAILABLE = True


Suggested change

FFTW_IS_AVAILABLE = True

PYFFTW_IS_AVAILABLE = True

What happens when pyfftw is available but fftw is not installed? A user might think that they are both the same thing and that fftw is installed with pyfftw. We probably want a way to detect this to be safe

It is okay to set PYFFTW_IS_AVAILABLE assuming that we also check that fftw is also installed

What happens when pyfftw is available but fftw is not installed? We probably want a way to detect this to be safe

It is okay to set PYFFTW_IS_AVAILABLE assuming that we also check that fftw is also installed

Got the point 👍

NimaSarajpoor · 2026-02-17T05:43:47Z

stumpy/sdp.py

+    Notes
+    -----
+    The class maintains internal caches of FFTW objects to avoid redundant planning
+    operations when called multiple times with similar-sized inputs and parameters.


Maybe add:

When `planning_flag=="FFTW_ESTIMATE"`, there will be no planning operation. However, caching FFTW objects is still beneficial as the overhead of creating those objects can be avoided.

But planning_flag is not an input parameter to this class and seems irrelevant? i.e., the user has no control over it

Okay, it looks like you can __call__ the class instance with the planning_flag

NimaSarajpoor · 2026-02-17T05:55:12Z

tests/test_sdp.py

        if func_name.endswith("sliding_dot_product"):
            out.append(func_name)

+    if sdp.FFTW_IS_AVAILABLE:  # pragma: no cover


Here, in test_sdp.py, I am checking sdp.FFTW_IS_AVAILABLE instead of adding

try: import pyfftw FFTW_IS_AVAILABLE = True except ImportError: FFTW_IS_AVAILABLE = False

Note that if I use the try-except block here, flake8 will complain that pyfftw is imported but not used.

seanlaw

@NimaSarajpoor I've left some comments for you to address. I think we can afford to be clearer since all of this pyfftw stuff will be hard to maintain. We should probably be as verbose (and add more comments cross referencing their docs as possible). I'll do another few passes after you've responded and made modifications. I think this pyfftw stuff needs to be crystal clear

seanlaw · 2026-02-17T12:20:55Z

stumpy/sdp.py

+try:  # pragma: no cover
+    import pyfftw
+
+    FFTW_IS_AVAILABLE = True


What happens when pyfftw is available but fftw is not installed? A user might think that they are both the same thing and that fftw is installed with pyfftw. We probably want a way to detect this to be safe

seanlaw · 2026-02-17T13:37:22Z

stumpy/sdp.py

+try:  # pragma: no cover
+    import pyfftw
+
+    FFTW_IS_AVAILABLE = True


It is okay to set PYFFTW_IS_AVAILABLE assuming that we also check that fftw is also installed

seanlaw · 2026-02-17T13:38:31Z

stumpy/sdp.py

+    A class to compute the sliding dot product using FFTW via pyfftw.
+
+    This class uses FFTW (via pyfftw) to efficiently compute the sliding dot product
+    between a query sequence Q and a time series T. It preallocates arrays and caches


"a query sequence, Q, and a time series, T"

seanlaw · 2026-02-17T13:44:33Z

stumpy/sdp.py

+        Preallocated complex-valued array for FFTW computations.
+
+    rfft_objects : dict
+        Cache of FFTW forward transform objects, keyed by


"transform object swith (next_fast_n, n_threads, planning_flag) as lookup keys"

The arguments for these keys seem opaque to the user of the class because they would have no control over them

The arguments for these keys seem opaque to the user of the class because they would have no control over them

n_threads and planning_flag are parameters of __call__. I need to add the Methods section to the docstring of class and add a description there. The tricky one is next_fast_n since users need to calculate that based on n=len(T).

seanlaw · 2026-02-17T13:46:04Z

stumpy/sdp.py

+    Notes
+    -----
+    The class maintains internal caches of FFTW objects to avoid redundant planning
+    operations when called multiple times with similar-sized inputs and parameters.


But planning_flag is not an input parameter to this class and seems irrelevant? i.e., the user has no control over it

seanlaw · 2026-02-17T13:49:46Z

stumpy/sdp.py

+    Notes
+    -----
+    The class maintains internal caches of FFTW objects to avoid redundant planning
+    operations when called multiple times with similar-sized inputs and parameters.


Okay, it looks like you can __call__ the class instance with the planning_flag

seanlaw · 2026-02-17T14:00:37Z

stumpy/sdp.py

+            )
+            self.rfft_objects[key] = rfft_obj
+        else:
+            rfft_obj.update_arrays(real_arr, complex_arr)


What does this do? It looks like it's a pyfftw thing

seanlaw · 2026-02-17T14:06:29Z

stumpy/sdp.py

+        # Get or create FFTW objects
+        key = (next_fast_n, n_threads, planning_flag)
+
+        rfft_obj = self.rfft_objects.get(key, None)


Would it make sense to combine both:

key = (next_fast_n, n_threads, planning_flag) rfft_obj = self.rfft_objects.get(key, None) irfft_obj = self.irfft_objects.get(key, None) if rfft_obj is None or irfft_obj is None: rfft_obj = pyfftw.FFTW( input_array=real_arr, output_array=complex_arr, direction="FFTW_FORWARD", flags=(planning_flag,), threads=n_threads, ) irfft_obj = pyfftw.FFTW( input_array=complex_arr, output_array=real_arr, direction="FFTW_BACKWARD", flags=(planning_flag, "FFTW_DESTROY_INPUT"), threads=n_threads, ) self.rfft_objects[key] = rfft_obj self.irfft_objects[key] = irfft_obj else: rfft_obj.update_arrays(real_arr, complex_arr) irfft_obj.update_arrays(complex_arr, real_arr)

This also assumes that things MUST be done in pairs. There shouldn't be a case where rfft_obj is defined but irfft_obj is not defined, right? That would be suspect

seanlaw · 2026-02-17T14:09:23Z

stumpy/sdp.py

+        real_arr[:n] = T
+        real_arr[n:] = 0.0
+        rfft_obj.execute()  # output is in complex_arr
+        complex_arr_T = complex_arr.copy()


This line is confusing. It's not clear what complext_arr_T is and why a copy would make complex_arr be related to T

seanlaw · 2026-02-17T14:10:41Z

stumpy/sdp.py

+        rfft_obj.execute()  # output is in complex_arr
+
+        # RFFT(T) * RFFT(Q)
+        np.multiply(complex_arr, complex_arr_T, out=complex_arr)


Here, your comment says RFFT(T) * RFFT*Q) but it is confusing to see complex_arr, complex_arr_T. So:

Both are complex and not "real"

Q is not referenced

NimaSarajpoor added 2 commits February 17, 2026 00:08

add pyfftw sdp and check for fftw/pyfftw

87dfcdb

fixed coverage

ae90507

NimaSarajpoor requested a review from seanlaw as a code owner February 17, 2026 05:38

NimaSarajpoor commented Feb 17, 2026

View reviewed changes

seanlaw requested changes Feb 17, 2026

View reviewed changes

Conversation

NimaSarajpoor commented Feb 17, 2026

Uh oh!

gitnotebooks bot commented Feb 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanlaw left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NimaSarajpoor Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

seanlaw left a comment •

edited

Loading

NimaSarajpoor Feb 18, 2026 •

edited

Loading