merge Fourier method into main

[MuellerSeb]

Since the 1.0 tag was created from this branch, it should be merged into main.

[adamreichold]

It is not trivial due to the new Bound-API, but you may want to consider upgrading to pyo3 0.22 as it significantly reduces Python-to-Rust call overheads.

[LSchueler]

Hi Adam, it's great to hear from you! Yeah, I realized that updating the version created a few problems and I wanted to publish the new function as quickly as possible. But now, I should have some time to see how to upgrade pyo3.

[LSchueler]

The benchmarks are inconclusive, but there might actually be a slight improvement 👍

[adamreichold]

This will add a significant cost for initializing a new thread pool for each and every call, i.e. via the thread start-up cost.

Maybe this could be changed to use the global/default thread pool if num_threads is None? Especially as rayon::current_num_threads is not the default (that would be RAYON_NUM_THREADS or std::thread::available_parallelism otherwise), but the number of threads in the global thread pool in contrast to the function documentation. (So basically it will use the number of threads chosen when the global pool was initialized which of course will be the default if it was initialized implicitly/automatically.)

Maybe something like

fn with_num_threads<F, R>(num_threads: Option<usize>, f: F) where F: FnOnce() -> R {
  if let Some(num_threads) = num_threads {
    ThreadPoolBuilder::new(num_threads).build().unwrap().install(f)
  } else {
    f()
  }
}

[LSchueler]

Thanks for pointing that out and thanks for the suggestion. I'll take a closer look soon.

[adamreichold]

Since the accumulator is a scalar and hence cheap to copy/reduce, I wonder whether this would actually benefit from using a par_fold instead of the sequential fold?

[LSchueler]

And thanks again :-) I'll run a few benchmarks soon to see, if you are right.

[LSchueler]

Interesting, the par_fold version is about 50% slower in my benchmarks.

[adamreichold]

Quite possible that the overhead is unhelpful for the inner loop which would otherwise benefit from auto-vectorization. Using with_min_len might be a nice compromise between granularity and overhead, but it does add another tunable.

[LSchueler]

Oh man, the weeks before my holidays where pretty stressful and I had to constantly jump between different things. But still, creating the tag from this branch?! Thanks for catching this!