Conversation
|
Review these changes at https://app.gitnotebooks.com/stumpy-dev/stumpy/pull/1132 |
There was a problem hiding this comment.
@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 \ |
There was a problem hiding this comment.
It seems that fftw-wisdom is not a MUST have: For instance, see the following Github Acrtions Run:
- https://github.com/NimaSarajpoor/automate/actions/runs/22208636175/job/64241168312
- https://github.com/NimaSarajpoor/automate/actions/runs/22186869860/job/64163022216
I can see thatfftw-wisdom is not available, and pyfftw is installed and tests are passing.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Can you post an issue to the pyfftw repo?
There was a problem hiding this comment.
Can you post an issue to the
pyfftwrepo?
Done. See: pyFFTW/pyFFTW#430
There was a problem hiding this comment.
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?"
This is to address
PR 3described in #1118 (comment). Have copied the corresponding notes below: