Add pyfftw sdp

[NimaSarajpoor]

This is to address PR 3 described in #1118 (comment). Have copied the corresponding notes below:

1. Add _PYFFTW_SLIDING_DOT_PRODUCT to sdp.py
2. Add unit tests that demonstrate that _PYFFTW_SLIDING_DOT_PRODUCT matches the results of naive_rolling_window_dot_product
3. Handle the test.sh check_fftw_pyfftw stuff here too

[gitnotebooks]

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

[seanlaw]

@NimaSarajpoor I've left some comments for you to address. I think we can afford to be clearer since all of this pyfftw stuff will be hard to maintain. We should probably be as verbose (and add more comments cross referencing their docs as possible). I'll do another few passes after you've responded and made modifications. I think this pyfftw stuff needs to be crystal clear

[NimaSarajpoor]

@seanlaw
This PR should be ready to be merged if there are no concerns.

[seanlaw]

@NimaSarajpoor This might be a distraction but a (very, very small) part of me was wondering if we should be using a closure instead of a class:

https://realpython.com/python-closure/

I don't know if it makes things easier/harder to read. Given all of the array allocations, maybe a class is the right thing to use. I just wanted to bring it to your attention since I'm not sure we're getting the full benefits of a class since:

1. None of the class attributes are actually being accessed externally
2. We don't really have an external method (except __call__) but we do want to make it a function

Just something to consider. No pressure though.

[NimaSarajpoor]

We don't really have an external method (except call) but we do want to make it a function

This is something that bothered me too but thought the reason for being bothered is just my lack of experience and, also, I didn't know/think about an alternative option.

but a (very, very small) part of me was wondering if we should be using a closure instead of a class:
https://realpython.com/python-closure/

Thanks for sharing this! I will go through this and try to modify the script. Let's see how it will look like.

[NimaSarajpoor]

@seanlaw
The code is modified to use closure. I pushed the changes so that you can see it too. It looks readable to me. Also, this way the user cannot change the attributes that are used internally. I think that should be a good thing. Right?

[seanlaw]

The code is modified to use closure. I pushed the changes so that you can see it too. It looks readable to me.

Yeah, it feels pretty natural! There is one thing that still bothers me and that is "how do you change/reset max_n after the function has been created?"

The natural thing to do is to overwrite the existing sdp._pyfftw_sliding_dot_product with a new call to _make_pyfftw_sliding_dot_proudct along with your desired max_n but feels a bit cumbersome (or hard to remember). Did you have any thoughts about this?

It feels like we're very close.

Also, this way the user cannot change the attributes that are used internally. I think that should be a good thing. Right?

Indeed. This is partially why I didn't like using the class approach!

[NimaSarajpoor]

@seanlaw

There is one thing that still bothers me and that is "how do you change/reset max_n after the function has been created?"

Did you have any thoughts about this?

I did not think about this scenario. I am now thinking in what scenario(s) a user might want to change max_n. Maybe they need to process much larger arrays at some point and they want to increase max_n to a certain value once to cover those lengths (?) or maybe they already processed some large arrays and now there is no need to have large preallocated arrays.

We can define a new function inside _make_pyfftw_sliding_dot_product and then return that object too. Then users can leverage that function to change values. Not sure if that is a good approach though as now users need to track two different objects returned by _make_pyfftw_sliding_dot_product. The option class might be better if we are going to end up with having more than one object returned by _make_pyfftw_sliding_dot_product.

[NimaSarajpoor]

@seanlaw

There is one thing that still bothers me and that is "how do you change/reset max_n after the function has been created?"

Did you have any thoughts about this?

I did not think about this scenario. I am now thinking in what scenario(s) a user might want to change max_n. Maybe they need to process much larger arrays at some point and they want to increase max_n to a certain value once to cover those lengths (?) or maybe they already processed some large arrays and now there is no need to have large preallocated arrays.

We can define a new function inside _make_pyfftw_sliding_dot_product and then return that object too. Then users can leverage that function to change values. Not sure if that is a good approach though as now users need to track two different objects returned by _make_pyfftw_sliding_dot_product. The option class might be better if we are going to end up with having more than one object returned by _make_pyfftw_sliding_dot_product.

Okay... on second thought, I think it should be okay to allow the returned callable object has a "callable feature" for resetting max_n. I think most users do not need to reset the pre-allocated arrays, and this design should be okay. See the changes in the last commit.

[seanlaw]

I think it should be okay to allow the returned callable object has a "callable feature" for resetting max_n

Can you please show me a complete example of how this works (i.e., from instantiating a new object to changing the max_n)?