Add test for challenger and warning

[NimaSarajpoor]

This PR is to add test for the modules in /sdp/

[NimaSarajpoor]

@seanlaw
Once you find some time, can you please review my comments and add yours?

[NimaSarajpoor]

[Update]
Moved some functions to utils.py, and the unit testing now covers all modules that have setup & sliding_dot_product.

[seanlaw]

For testing, it would be good to also test non-power-of-2 lengths. So, say, p-1 and p+1 or large-ish prime numbers

I feel uneasy only testing perfect powers of two

[NimaSarajpoor]

@seanlaw

I feel uneasy only testing perfect powers of two

Right! I will revise the unit testing.

[NimaSarajpoor]

@seanlaw

For testing, it would be good to also test non-power-of-2 lengths. So, say, p-1 and p+1 or large-ish prime numbers.
I feel uneasy only testing perfect powers of two

Added test and got assertion error locally for one of the sdp's modules. Will add GitHub Actions to expose the error here first, and then work on fixing it here (or in another PR)

[seanlaw]

Let's try to fix it here

[NimaSarajpoor]

@seanlaw
I've addressed most of the comments.

(1) Changed branches: '**' to branches: add_test for now so that I can still see GitHub Actions in my fork. Just before merge, will change it to branches: main
(2) Couldn't figure out how to add name: to the part where we set default bash to shell: bash -el {0}. Added some comments only.

[NimaSarajpoor]

@seanlaw
I also added a few comments for myself. Can you please look at them and let me know what you think?

[seanlaw]

@NimaSarajpoor I left a few comments

[seanlaw]

@NimaSarajpoor I added some additional comments (more thorough)

[NimaSarajpoor]

@seanlaw
I addressed the comments, and did some minor changes. If there is nothing left to be addressed, I think it is ready to be merged.

---

Btw, regarding long running time of coverage that we faced before. I was supposed to check the performance of njit-vs-naive when njit is off. I did and noticed that njit is considerably slower. Going to attach the timing here just in case.
timing.csv

This is obtained by setting the env var NUMBA_DISABLE_JIT to 1, and then revising timing.sh as follows:

rm -rf sdp/__pycache__
./timing.py -niter 10 -pmin 2 -pmax 12 pyfftw_sdp njit_sdp challenger_sdp  > timing.csv
rm -rf sdp/__pycache__

[seanlaw]

Btw, regarding long running time of coverage that we faced before. I was supposed to check the performance of njit-vs-naive when njit is off. I did and noticed that njit is considerably slower. Going to attach the timing here just in case.
timing.csv

What does this mean for coverage testing though? How long are the unit tests and coverage tests taking now?

[seanlaw]

Thanks for working on this @NimaSarajpoor!

[NimaSarajpoor]

@seanlaw

What does this mean for coverage testing though?

we are good now as the current tests are only for a small number of queries, with short length. I just did not want to leave that question unanswered.

How long are the unit tests and coverage tests taking now?

The unit test and coverage step takes less than 1min in each platform.