Re: Multithreaded FFTW3 in octave?

[Andreas Weber]

On 18.12.2012 05:08, Mike Miller wrote:

On Mon, Dec 17, 2012 at 1:04 PM, Jordi Gutiérrez Hermoso
<address@hidden> wrote:

On 16 December 2012 19:05, Andreas Weber <address@hidden> wrote:

Hi Mike, I've attached a "first shot" patch which

I like this patch. At a glance, it looks like a very good addition.
I'll wait for Mike's comments. If he doesn't merge it in within a
couple of days, I will.

Looks good overall, thanks for working on this. I just tested briefly
and it works for me as expected, for the most part. I do have a few
comments.

Hi Mike, I appreciate your comments,

The configure changes don't look quite right to me. I wouldn't make
the HAVE_FFTW3_THREADS condition dependent on a separate fftw3_threads
option. The way you have it here I don't think I can configure
"--with-fftw3=somefftwlib" and have it detect whether some
FFTW-compatible library has the threading API or not. 

Is there any "FFTW-compatible library"? I don't know one and as I kan see this also wasn't possible before my changes.

And you do not want to be able to disable multithreading with "--disable-fftw-threads?
So just check if its possible to compile and link with multithreaded libs, if not try the single threaded,
if that fails try the FFTPACK?

It's also
linking with both fftw3_threads and fftw3 (non-threaded).

From the FFTW3 manual:
...programs using the parallel complex transforms should be linked with -lfftw3_threads -lfftw3 -lm on Unix...
also  nm -D --defined-only libfftw3_threads.so doesn't show any DFT function.
Please correct me if I'm wrong but I think ist correct to link with both.

 I'd rather
see it test the existing FFTW3_LIBS and FFTW3F_LIBS whether they
implement one of the threading functions.

See above

I'm not sure off the top of my head what's the cleanest way to do it
that way *and* make it automatically use the libfftw3_threads library
if it's installed. And how to express whether to use libfftw3_threads
or libfftw3_omp? Worst case, we could require users to configure with
"--with-fftw3=fftw3_threads --with-fftw3f=fftw3f_threads", this has
pros and cons. I think I prefer that for now, it's the easiest and
safest and we can always change it later.

I can't see any benefit from doing this and want to ask some autotools pro for that.

I would set the default number of threads to 1 at startup. Matches the
default behavior of both the non-threaded and threaded FFTW libraries.

Okay, I thought a dual core is standard nowadays and users would benefit from this even when they didn't notice the change that they now can set the number of threads with fftw("threads",...)

Another way could be to set the default number of threads to the number of available cores on startup but,
1.) I don't like such type of "hidden automation"
2.) We first would have to implement some platform independent way to get the number of available CPU cores

There is some trailing whitespace on a few lines that should be removed.

Okay, done that

I already fixed the method / do_method bug recently on the stable
branch (http://hg.savannah.gnu.org/hgweb/octave/rev/f63a4f23bfe7), I
think it simply hasn't been merged into the dev branch yet, so don't
include that in this patch.

Ah I didn't notice that you've fixed that on stable. I stumbled over this problem some days ago and had to fix it for my tests with fftw.
When do you merge this from stable into default?

Other than that, looks real good.

Thank you very much. I want to talk with Jordi on IRC gerading some points and submit then a new patch.
And I appreciate any further comments.

I've further made changes that the "threads" getter and setter is only compiled if defined (HAVE_FFTW3F_THREADS)
and the plan only gets discarded when there is a change in the number of threads.

Regards, Andy