Skip to content

Add pyfftw sdp#1132

Open
NimaSarajpoor wants to merge 10 commits intostumpy-dev:mainfrom
NimaSarajpoor:add_pyfftw_sdp
Open

Add pyfftw sdp#1132
NimaSarajpoor wants to merge 10 commits intostumpy-dev:mainfrom
NimaSarajpoor:add_pyfftw_sdp

Conversation

@NimaSarajpoor
Copy link
Copy Markdown
Collaborator

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

  1. Add _PYFFTW_SLIDING_DOT_PRODUCT to sdp.py
  2. Add unit tests that demonstrate that _PYFFTW_SLIDING_DOT_PRODUCT matches the results of naive_rolling_window_dot_product
  3. Handle the test.sh check_fftw_pyfftw stuff here too

@gitnotebooks
Copy link
Copy Markdown

gitnotebooks bot commented Feb 17, 2026

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

Copy link
Copy Markdown
Contributor

@seanlaw seanlaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

{
# If `ray` command is not found then generate a .coveragerc_override file
if ! command -v ray &> /dev/null
if ! command -v fftw-wisdom &> /dev/null \
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that fftw-wisdom is not a MUST have: For instance, see the following Github Acrtions Run:

I can see thatfftw-wisdom is not available, and pyfftw is installed and tests are passing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, what should we do?

Copy link
Copy Markdown
Collaborator Author

@NimaSarajpoor NimaSarajpoor Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove the check command -v fftw-wisdom from ./test.sh.

There is still something that I cannot figure out though. If pyfftw can be imported with no error, does that mean it can compute fft for any dtype listed in the Schema table in pyfftw doc? I think the answer is "No". Because I noticed the issue pyFFTW/pyFFTW#70 says:

In some Linux distributions it is common to have only the fftw3 library and not fftw3f. PyFFTW should detect that it is not available and work anyways (just not accept float32/single-precision inputs).

In addition, what I infer from these lines of code in pyfftw is that pyfftw might be installed successfully based on some available fftw* but it may not work for all supported dtypes. In our code, dtype is set to np.float64. One way to check is to use wisdom returned by pyfftw.export_wisdom():

Return the FFTW wisdom as a tuple of strings.
The first string in the tuple is the string for the double precision wisdom, the second is for single precision, and the third for long double precision. If any of the precisions is not supported in the build, the string is empty.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you post an issue to the pyfftw repo?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you post an issue to the pyfftw repo?

Done. See: pyFFTW/pyFFTW#430

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to this comment in the ongoing discussion in pyFFTW/pyFFTW#430:

FFTW is included in wheels available on PyPI so users of these wheels do not need to install FFTW.

The dependencies listed in the README are only interesting when you want to build pyfftw from source.

This Github Action run failed when it tried to install pyfftw on macOS from pypi and the error says:

      /var/folders/t5/f77_gwnj6p95qxy9py3fckx00000gn/T/pyfftw-ffhdxcpf/None.c:1:10: fatal error: 'fftw3.h' file not found
          1 | #include <fftw3.h>
            |          ^~~~~~~~~
      1 error generated.
      WARNING:__main__:Compilation error: command '/usr/bin/clang' failed with exit code 1
      error: Could not find the FFTW header 'fftw3.h'

      hint: This error likely indicates that you need to install a library that provides "fftw3.h" for `pyfftw@0.15.1`

And the following explanation was provided:

uv tries to build pyfftw from source, what fails since fftw is not installed and in particular fftw3.h is not available. But uv should not actually try to build pyfftw since it should just use the wheel available on PyPI. I don't understand why uv thinks that it needs to build pyfftw.


I don't understand why uv thinks that it needs to build pyfftw.

My understanding is that pip/uv will build from source if they cannot install wheel. One needs to check why the installation didn't go through the default "wheel" route. But I think we now have the info to say the following statement:

"IF pyfftw installation becomes successful, it means the fftw dependency was already taken care of. And if it builds it from source when fftw is not available, there will be an error."


I am currently waiting to get response for the following questions I asked here:

(1) [If pyfftw can be installed successfully either via wheel or from source.... and] If I can do import pyfftw without getting any error, does it guarantee that all features (all precisions, multi-threading, etc??) are supported? (I think the answer is "NO")

(2) If the answer to (1) is "NO", then the question is: "What features might be affected? and how can we check that?"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants