search

mborgerding / kissfft

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

Small FFT sizes fail with OPENMP #26

Closed JulienMaille closed 5 years ago

JulienMaille commented 5 years ago

I while ago I had some weird issue when using kissfft in combination with OpenMP. At that time I found a few threads mentioning the same issue https://sourceforge.net/p/kissfft/discussion/278332/thread/b2809c8b/

and ended up with that fix: if (fstride==1 && p<=5 && m!=1) //Fix from http://sourceforge.net/p/kissfft/bugs/10/

Now that the link above is dead, I'm not sure if that patch is still needed.

mborgerding commented 5 years ago

Looks like this is still an issue. Do you have a fix?

Steps to reproduce.

markb@galois:~/kissfft/test$ make CFLAGADD=-fopenmp st_float
markb@galois:~/kissfft/test$ ./st_float 10
Segmentation fault
JulienMaille commented 5 years ago

Well the workaround is if (fstride==1 && p<=5 && m!=1) but this is not a clean fix. https://github.com/mborgerding/kissfft/blob/1837c1e4fdc59a5ca02ef96b505c23ac38343137/kiss_fft.c#L249 It think you discussed that issue in http://sourceforge.net/p/kissfft/bugs/10/ but unfortunately that link is dead. Maybe you still have access to it?

JulienMaille commented 5 years ago

Looks like @MoonchildProductions forked and fixed a couple OpenMP related bugs a while ago. https://github.com/MoonchildProductions/kissfft/commit/e0e32d54b4c9834d8a47467c1bf81cc73e7804dc https://github.com/MoonchildProductions/kissfft/commit/ebdf89db029a63ded7a7c3492e637c0efc578891 https://github.com/MoonchildProductions/kissfft/commit/fa1bf9189dc84f960d4e56c0bed25d961c0ccb76

mborgerding commented 5 years ago

Cool. I'll look into cherry-picking his commits.

JulienMaille commented 5 years ago

Have you had time to look at his commits?

mborgerding commented 5 years ago

The first of those commits fixes the "Steps to reproduce" above. It is not clear to me what the others are fixing, so I am hesitant to bring them in. Perhaps @wolfbeast could elaborate.

wolfbeast commented 5 years ago

It's explained in https://github.com/MoonchildProductions/kissfft/issues/2

The third commit is to correct some brace confusion in the second.