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]

Not clear why this condition is needed ?

[NimaSarajpoor]

According to my exploration, replacing this with scipy's function _calc_oa_lens results in a considerable performance hit. I am passing real=True in next_fast_len(math.ceil(opt_size), real=True) in the function here; however, the scipy's function (internally) uses the default real=False.

[NimaSarajpoor]

I've checked the performance of the challenger with real=False to see its impact on the performance. As shown in the figure below, challenger (in which real=True is passed to next_fast_len) shows a better performance.

rm -rf sdp/__pycache__
./timing.py -timeout 1.0 -pmin 7 -pmax 20 pyfftw challenger challenger_real_false > timing.csv
rm -rf sdp/__pycache__