mreineck / pocketfft

Fork of https://gitlab.mpcdf.mpg.de/mtr/pocketfft to simplify external contributions
BSD 3-Clause "New" or "Revised" License
71 stars 34 forks source link

pocketfft is hanging with mingw-w64 #1

Closed carlkl closed 2 years ago

carlkl commented 2 years ago

The first problem I encountered with pocketfft and mingw-w64 was:

I had to add: #define drand48() (((double)rand())/((double)RAND_MAX)) to pocketfft_demo.cc to compile it with g++ or MSVC cl.

g++ pocketfft_demo.cc -std=c++14 -o pocketfft_demo.exe -> is hanging

g++ pocketfft_demo.cc -DPOCKETFFT_NO_MULTITHREADING -std=c++14 -o pocketfft_demo_NOMT.exe -> is working as expected

The Microsoft cl compiled version works as well.

I used msys2 g++ with posix threads enabled, hence with pthread and C11 thread support.

mreineck commented 2 years ago

Thanks for the report!

The drand48 issue is probably due to a missing #include <cstdlib>; I'll fix that as soon as possible!

Concerning the hanging executable I probably can't help much, because I do not have access to a Windows machine. You could try adding -pthread to the compilation command, but I'm not sure that this is the problem.

BTW if you want to look at performance, please enable compiler optimizations, otherwise the code will be very, very slow :-)

mreineck commented 2 years ago

OK, drand48 is only available on POSIX-compliant systems. I'll find a workaround.

carlkl commented 2 years ago

The workaround with #define drand48() (((double)rand())/((double)RAND_MAX)) is sufficient. This works with mingw-w64 and with MSVC and should't produce side effects.

#if defined _WIN32
#define drand48() (((double)rand())/((double)RAND_MAX))
#endif

Unfortunately applying -pthread doesn't help. The problem is that mingw-64 is defining pthread_atfork as an empty function as Windows can't fork.

Now I found by accident that this seems to work:

g++ pocketfft_demo.cc -DPOCKETFFT_PTHREADS -std=c++14 -o pocketfft_g++.exe

But I have to idea why this works, as pthread_atfork() performes exactly nothing in case of mingw-w64.

mreineck commented 2 years ago

I don't understand all the details of that code; it was written by @peterbell10 and he has much deeper insight into multithreading. But I'm also surprised that registering a function that does nothing (and is never called) can improve things ...

carlkl commented 2 years ago

The mingw-w4 pthread stuff is defined here: https://github.com/mirror/mingw-w64/blob/master/mingw-w64-libraries/winpthreads/include/pthread.h

matthew-brett commented 2 years ago

@carlkl - I do not get a hang from:

g++ pocketfft_demo.cc -std=c++14 -o pocketfft_demo.exe
.\pocketfft_demo.exe

My g++ is g++.exe (Rev2, Built by MSYS2 project) 11.2.0, but I imagine your version is the same.

carlkl commented 2 years ago

@peterbell10, please can you comment on this issue? It seems that threads are effectively unsupported on Windows right now. Could we possibly use another pthread pool usuable from mingw-w64 gcc?

mreineck commented 2 years ago

I'm a bit confused now ... didn't you mention that specifying -DPOCKETFFT_PTHREADS fixed the problem for you (even though we currently don't know why)?

carlkl commented 2 years ago

Yes, it fixed the problem sometimes. The safe option is -DPOCKETFFT_NO_MULTITHREADING.

However, scipy.fft has the parallelization function based on the pocketfft pthread pool. This functionality has never worked under Windows.

I will close this issue and open a new one.

mreineck commented 2 years ago

Yes, it fixed the problem sometimes. The safe option is -DPOCKETFFT_NO_MULTITHREADING.

OK, I wasn't aware that this fix didn't work reliably.

However, scipy.fft has the parallelization function based on the pocketfft pthread pool. This functionality has never worked under Windows.

I see, I wasn't aware of that either.

Would it make sense to open this issue for scipy then? It's not that I want to delegate responsibility, but I expect it might be seen by a lot more knowledgeable people there.

carlkl commented 2 years ago

see: https://github.com/mreineck/pocketfft/issues/2

peterbell10 commented 2 years ago

However, scipy.fft has the parallelization function based on the pocketfft pthread pool. This functionality has never worked under Windows.

I've never header this before and the multithreading functionality is tested in windows CI. Are you building SciPy from source with mingw or installing from a wheel?

rgommers commented 2 years ago

Are you building SciPy from source with mingw or installing from a wheel?

This is when building from source with a standard mingw-w64 toolchain, which we need for the Meson build (preferable to reimplementing the whole "glue MSVC and gfortran together" machinery that numpy.distutils has.

matthew-brett commented 2 years ago

As an extra comment - the threading does work as expected when building Scipy with MSVC - the hang is only (and apparently always) with Mingw-w64 compiled Scipy pocketfft.