search

mborgerding / kissfft

a Fast Fourier Transform (FFT) library that tries to Keep it Simple, Stupid
Other
1.39k stars 278 forks source link

Prepare for release 131.1 #58

Closed basilgello closed 3 years ago

basilgello commented 3 years ago

@mborgerding @jtojnar Here I will push commits for review

basilgello commented 3 years ago

I pushed the commit chain!

TODO to check / consider:

Also, @mborgerding please check CMake int32_t and int16_t Python tests: both do 100% fail regardless of Python / NumPy version:

basilgello commented 3 years ago

Later I will update documentation because i introduced many new Makefile and CMake configuration options.

jtojnar commented 3 years ago

Yes the .pc file should be installed and respect the PREFIX and other variables passed to make. Using sed should be okay.

mborgerding commented 3 years ago

Thanks for your work on this @basilgello @jtojnar . I'll take a look at the changes and the failing int tests tonight.

mborgerding commented 3 years ago

Also, @mborgerding please check CMake int32_t and int16_t Python tests: both do 100% fail regardless of Python / NumPy version:

Can you provide commands that demonstrate this? This works on my machine:

mkdir build
cd build
cmake ..
make
make test
basilgello commented 3 years ago

int16_t:

mkdir build && cd build && cmake -DKISSFFT_DATATYPE=int16_t .. && make && make test && cd .. && rm -rf build

int32_t:

mkdir build && cd build && cmake -DKISSFFT_DATATYPE=int32_t .. && make && make test && cd .. && rm -rf build

basilgello commented 3 years ago

Both int type tests fail inside test/testkiss.py : for int16_t, the fft_int16_t returns zero-dimensional array that is failed to transform and for int32_t, SNR calculated by fft_int32_t is less than the value calculated by numpy.

basilgello commented 3 years ago

Interesting that running the same Python tests from Makefile succeeds :) I woke up with that fact bogging my mind, gonna check why CMake invocation fails.

EDIT: looks like DATATYPE is not passed correctly by CMake build. But even with it, running "proper" Makefile yields:

int16_t :

make clean && make DATATYPE=int16_t all && make -C test DATATYPE=int16_t test

for i in `seq 1 1 1000`
do
  cd test; echo "=== $i ==="
  echo ""
  DATATYPE=int16_t LD_LIBRARY_PATH="$LD_LIBRARY_PATH:..:." ./testkiss.py
  _RET=$?
  echo ""
  cd ..
  [ $_RET -ne 0 ] && break
done
=== 89 ===

Testing multi-dimensional FFTs
../tools/fft_int16_t -n 2
SNR (compared to NumPy) : 78.6dB
../tools/fft_int16_t -n 3,4
SNR (compared to NumPy) : -0.8dB
xver= [[-1.8223773 +0.03095927j  2.21032589+2.1172731j  -0.07035792+1.06732496j
  -0.18454504+0.11418401j]
 [-2.58112351+0.08827616j -0.80299513-3.30159131j -0.3757195 -1.01454038j
  -1.92478413-0.01256612j]
 [ 0.98187482-1.62526194j  2.34471157-1.54612171j -5.0414784 -1.25918512j
   0.23294821+6.60562751j]]
x2= [[-1.82195502+0.03112888j  2.20978423+2.11676382j -0.06994842+1.06753746j
  -0.18457595+0.1142613j ]
 [-2.58040101+0.08862575j -0.80275887-3.30112613j -0.37574389-1.01443525j
  -1.92486343-0.01245155j]
 [-5.01760918-1.6245613j   2.34455397+4.45362712j  0.95913572-1.25907163j
   0.23291726+0.60536515j]]
err [[-4.22279633e-04-1.69611433e-04j  5.41655220e-04+5.09282534e-04j
  -4.09501264e-04-2.12502178e-04j  3.09102087e-05-7.72913559e-05j]
 [-7.22497766e-04-3.49590650e-04j -2.36260964e-04-4.65173855e-04j
   2.43932974e-05-1.05122741e-04j  7.93036045e-05-1.14565814e-04j]
 [ 5.99948400e+00-7.00640597e-04j  1.57601652e-04-5.99974884e+00j
  -6.00061412e+00-1.13493837e-04j  3.09498556e-05+6.00026236e+00j]]

int32_t :

make clean && make DATATYPE=int32_t all && make -C test DATATYPE=int32_t test

for i in `seq 1 1 1000`
do
  cd test; echo "=== $i ==="
  echo ""
  DATATYPE=int32_t LD_LIBRARY_PATH="$LD_LIBRARY_PATH:..:." ./testkiss.py
  _RET=$?
  echo ""
  cd ..
  [ $_RET -ne 0 ] && break
done
=== 105 ===

Testing multi-dimensional FFTs
../tools/fft_int32_t -n 4
SNR (compared to NumPy) : 109.9dB
../tools/fft_int32_t -n 2,4
SNR (compared to NumPy) : 116.1dB
../tools/fft_int32_t -n 3,3,3
SNR (compared to NumPy) : 126.5dB

=== 106 ===

Testing multi-dimensional FFTs
../tools/fft_int32_t -n 3
SNR (compared to NumPy) : 107.2dB
../tools/fft_int32_t -n 4,3
SNR (compared to NumPy) : 120.0dB
../tools/fft_int32_t -n 3,2,4
SNR (compared to NumPy) : 1.1dB
xver= [[[-2.78979234-3.22938157j -3.99565485+4.55492108j
    1.41446985-1.33486917j -2.85273063+1.95857005j]
  [-0.81288857+2.0363194j   0.52624015-0.56039184j
    3.98158321-2.22647709j -0.31736739-1.65983782j]]

 [[-1.74174266-4.62098827j -1.22956951-0.4086652j
    1.02626952-1.73753693j  1.62383759-1.06004693j]
  [ 2.14748395-1.35770027j  2.06055428+4.79676997j
   -2.24141367+2.1611268j  -2.50284891-2.38277957j]]

 [[-2.43470728+1.65642868j  6.54993315-0.31608477j
    2.9450289 -1.71071983j  2.04543293+0.14472321j]
  [ 2.15884788-1.61883326j  7.73033669+2.21171711j
   -0.48934682+2.15038617j  6.08740098-0.20594898j]]]
x2= [[[-2.78979233-3.22938157j -3.99565484+4.55492107j
    1.41446983-1.33486917j -2.85273062+1.95857003j]
  [-0.81288857+2.03631939j  0.52624016-0.56039184j
    3.9815832 -2.22647708j -0.31736738-1.65983781j]]

 [[-1.74174263-4.62098825j -1.22956951-0.40866521j
    1.02626952-1.73753693j  1.62383758-1.06004693j]
  [ 2.14748394-1.35770027j  2.06055428+4.79676996j
   -2.24141365+2.16112679j -2.50284889-2.38277956j]]

 [[-8.43470727+1.65642869j  0.54993314-0.31608477j
   -3.05497109-1.71071982j -3.95456708+0.14472321j]
  [-3.8411521 -1.61883325j  1.73033667+2.21171709j
   -6.4893468 +2.15038614j  0.08740097-0.20594898j]]]
err [[[-6.61234356e-09-1.72469594e-09j -1.31059785e-08+1.07110028e-08j
    1.65638430e-08-7.06902270e-09j -1.01020565e-08+1.57132303e-08j]
  [ 2.56909294e-09+7.06057524e-09j -8.04141675e-09-1.23396027e-09j
    1.23743065e-08-1.08704361e-08j -1.08044490e-08-7.76540632e-09j]]

 [[-3.47241575e-08-2.65108921e-08j -1.36585188e-09+1.06502233e-08j
    3.27327698e-09+4.10474987e-10j  8.30589264e-09+3.41915074e-09j]
  [ 8.69423511e-09+5.15957277e-09j  2.68851608e-12+1.63131215e-08j
   -2.15702691e-08+4.33100311e-09j -1.44628434e-08-4.90723417e-09j]]

 [[ 5.99999999e+00-4.84289386e-09j  6.00000002e+00-3.63885699e-09j
    5.99999999e+00-1.60032336e-08j  6.00000001e+00-8.95266331e-09j]
  [ 5.99999998e+00-8.65134120e-09j  6.00000002e+00+1.84211104e-08j
    5.99999998e+00+2.13273270e-08j  6.00000001e+00+6.17555904e-09j]]]

So here are still two problems: one is CMake not passing DATATYPE environment variable to testkiss.py and the issue with transforms itself. I will fix CMake now, but the transforms are something you should investigate :)

basilgello commented 3 years ago

So here are still two problems: one is CMake not passing DATATYPE environment variable to testkiss.py and the issue with transforms itself. I will fix CMake now

Well, I was wrong on CMake problem. The build/Testing/Temporary/LastTest.log states:

8/8 Testing: testkiss.py
8/8 Test: testkiss.py
Command: "/usr/bin/python3.9" "/tmp/kissfft/test/testkiss.py"
Directory: /tmp/kissfft/build/test
"testkiss.py" start time: Jan 28 06:10 UTC
Output:
----------------------------------------------------------
Testing multi-dimensional FFTs
../tools/fft_int16_t -n 2
Traceback (most recent call last):
  File "/tmp/kissfft/test/testkiss.py", line 138, in <module>
    main()
  File "/tmp/kissfft/test/testkiss.py", line 134, in main
    test_fft(dim)
  File "/tmp/kissfft/test/testkiss.py", line 74, in test_fft
    x2 = dofft(x, do_real)
  File "/tmp/kissfft/test/testkiss.py", line 121, in dofft
    return np.reshape(res, dims)
  File "<__array_function__ internals>", line 5, in reshape
  File "/usr/lib/python3/dist-packages/numpy/core/fromnumeric.py", line 299, in reshape
    return _wrapfunc(a, 'reshape', newshape, order=order)
  File "/usr/lib/python3/dist-packages/numpy/core/fromnumeric.py", line 58, in _wrapfunc
    return bound(*args, **kwds)
ValueError: cannot reshape array of size 0 into shape (2,)
<end of output>
Test time =   0.22 sec
----------------------------------------------------------
Test Failed.
"testkiss.py" end time: Jan 28 06:10 UTC
"testkiss.py" time elapsed: 00:00:00
----------------------------------------------------------

I clearly see DATATYPE is passed here but nonetheless the error.

basilgello commented 3 years ago

I pushed the next revision that passes the extended testsuite.

@mborgerding @jtojnar I am going to push one more revision preparing pkg-config in Makefile and reflect that in testsuite & documentation. If you want to run extended testsuite, invoke 'sh test/kissfft-testsuite.sh' from kissfft root directory and let it run for ~1h.

mborgerding commented 3 years ago

The kssfft-testsuite.sh seems fine on an Ubuntu 20.04 system, but has inconsistent failures under CentOS 8:

[/tmp/kissfft-testsuite]$ grep 'error while' -r|sort -u
test-make-double-shared-tools:test/testcpp: error while loading shared libraries: libkissfft_double.so.131: cannot open shared object file: No such file or directory
test-make-float-shared-tools:test/testcpp: error while loading shared libraries: libkissfft_float.so.131: cannot open shared object file: No such file or directory
test-make-int16_t-shared-tools:test/testcpp: error while loading shared libraries: libkissfft_int16_t.so.131: cannot open shared object file: No such file or directory
test-make-int32_t-shared-tools:test/testcpp: error while loading shared libraries: libkissfft_int32_t.so.131: cannot open shared object file: No such file or directory
test-make-simd-shared-tools:test/testcpp: error while loading shared libraries: libkissfft_simd.so.131: cannot open shared object file: No such file or directory
basilgello commented 3 years ago

Thanks for testing! I will spin a container with centos 6/7/8 and verify why it fails!

basilgello commented 3 years ago

In the next update:

basilgello commented 3 years ago

@mborgerding @jtojnar I finally pushed the release candidate.

In this release:

basilgello commented 3 years ago

If possible, please review before Feb 8th, so I can migrate 131.1.0 to Debian bullseye without too much hassle.

basilgello commented 3 years ago

The pkg-config files seem to work great with both CMake and GNU Make on Nix, both in a single prefix and different prefixes for devel files. Great job.

Thanks for improvements! :)

mborgerding commented 3 years ago

Thanks for all the work @basilgello @jtojnar! I'll look at merging this to master tonight or tomorrow.

mborgerding commented 3 years ago

What do you think about an aggregation header `"${CMAKE_INSTALL_INCLUDEDIR}/kissfft.h"? Something like

/*Force compatibility with the installed shared lib*/
#define kiss_fft_scalar float

/*basic C interface for mixed radix complex FFTs*/
#include <kissfft/kiss_fft.h>

/*real 1D FFTs */
#include <kissfft/kiss_fftr.h>

/*multidimensional complex*/
#include <kissfft/kiss_fftnd.h>

/*multidimensional real*/
#include <kissfft/kiss_fftndr.h>

#ifdef __cplusplus
// header-only C++ version 
#  include <kissfft/kissfft.hh>
#endif

Advantages:

mborgerding commented 3 years ago

I'd recommend 8f47a67f595a6641c566087bf5277034be64f24d which removes kfc.h, kissfft_i32.hh, and README as discussed above.

basilgello commented 3 years ago

Thanks for merging! Sorry for delay on reviewing the kfc.h and kissfft_i32.hh removal!

I do like idea of an aggregate kissfft.h. Removing kfc.h from installation is fine without ABI bump because the solib still provides the necessary functions.

Further step is (maybe) to rename public API functions to be able to use several datatypes within a single application, but it is a breaking change that will need some discussion & preparation.

basilgello commented 3 years ago

Also please create a tagged release whenever KFVER constants change. This is needed for distribution watchdogs like Debian's vcswatch to track the availability of new version automatically.

For example, see https://github.com/kodi-pvr/pvr.iptvsimple