Fixed #34 Customizing scipy's oaconvolve

[NimaSarajpoor]

This PR is to address #34.

[gitnotebooks]

Review these changes at https://app.gitnotebooks.com/stumpy-dev/sliding_dot_product/pull/35

[NimaSarajpoor]

./timing.py -timeout 1.0 -pmin 7 -pmax 24 pyfftw pocketfft_r2c_c2r scipy_oaconvolve challenger > timing.csv

# in timing.py, I change timeout to 5.0 when `len(T) >= 2^20`

The challenger is the customized version of scipy's oaconvolve.

Observations:

• The challenger outperforms scipy's oaconvolve
• For len(Q) <= 2 ^16 (and len(Q)>= 2^7), challenger outperforms the others for the most part.
• For len(Q) > 2^16, pocketfft outperforms the others for the most part.

For me, the important one is the first bullet point. Of the four optimization opportunities mentioned in this comment, I've addressed 1, 2, and 3 in this PR. The last item, which is about adjusting the number of multiplication for real-valued arrays, can be explored next.

[seanlaw]

As a gentle reminder, even if we can do things faster, we will never (??) remove the public scipy convolution functions from STUMPY because they should be our last resort fallback (in case the alternatives, that may use private functions, raise an error). Does that make sense?

[NimaSarajpoor]

Good reminder. It makes sense!!

[NimaSarajpoor]

This line needs to be modified if, at a later time, we decide to move the proposal to a new file (module).

[NimaSarajpoor]

./timing.py -timeout 1.0 -pmin 7 -pmax 24 pyfftw pocketfft_r2c_c2r scipy_oaconvolve challenger > timing.csv

# in timing.py, I change timeout to 5.0 when `len(T) >= 2^20`

@seanlaw
"Challenger" seems to be the winner for most cases, and I think it is worth it to include it. What do you think? Also, can you please review the script? I've made major changes. The private objects are now only r2c and c2r. IMO, the script looks cleaner now.

[seanlaw]

@NimaSarajpoor I've left some comments but would still like another pass after you've cleaned things up further

I do agree that, for the most part, things look clean. I think it still lacks clarity as to what is happening or why the logic is coded in this way

[NimaSarajpoor]

@seanlaw
I think this is ready for another round of review. Note that:

(1) I intentionally left a few things to be addressed in a different PR later for the function _compute_block_size. This can help us see the impact of those changes once the current PR is merged.

(2) I thought about refactoring '_pocketfft_circular_convolve_block' and '_pocketfft_valid_convolve'. However, I couldn't find a clean way to do it. Maybe we need to create a new function and those both functions need to call that common function (??)