Fix #769 fix name and order of variables

[NimaSarajpoor]

No description provided.

[NimaSarajpoor]

In #769, @seanlaw pointed out that:

T_A should always correspond to Q and, vice verse, with T_B and T. Although, I wonder if it is confusing when we have functions that only take T and m but no Q? In that case, T has no direct relation to T_B

I think it would be reanable to think about this as follows:

• sometimes we just have a time series T and we are interested in finding its [sliding] mean, std, etc.
• sometimes we have the time series T, but we also have another time series / sequence Q which is our query

@seanlaw
What do you think? For now, I just revised stumpy/stump.py (and I had to do minor changes for two more modules). I will go through the other modules one by one, even the ones that were changed because of the changes in stump.py. If you are happy with this, we can start applying similar changes to other modules where appropriate.

[seanlaw]

What do you think?

I'm not sure I understand.

I think it would be reanable to think about this as follows:
sometimes we just have a time series T and we are interested in finding its [sliding] mean, std, etc.
sometimes we have the time series T, but we also have another time series / sequence Q which is our query

These sounds like true statements but I don't see how this is related to the code. Can you provide more context? At the end of the day, Q should really be a single subsequence (either 1D or multi-dimensional) but it should not contain more than one subsequence

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (286cb17) 99.14% compared to head (ff781f6) 99.14%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #889   +/-   ##
=======================================
  Coverage   99.14%   99.14%           
=======================================
  Files          83       83           
  Lines       14206    14206           
=======================================
  Hits        14084    14084           
  Misses        122      122           

Files Changed	Coverage Δ
stumpy/aamp.py	100.00% <ø> (ø)
stumpy/aamped.py	100.00% <ø> (ø)
stumpy/scraamp.py	100.00% <ø> (ø)
stumpy/scrump.py	100.00% <ø> (ø)
stumpy/stump.py	100.00% <ø> (ø)
stumpy/stumped.py	100.00% <ø> (ø)
stumpy/gpu_aamp.py	100.00% <100.00%> (ø)
stumpy/gpu_stump.py	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[NimaSarajpoor]

What do you think?

I'm not sure I understand.

I wanted to know if it is okay [from your perspective] to use the name Q instead of T_A. But, I think I get my answer when you said:

At the end of the day, Q should really be a single subsequence (either 1D or multi-dimensional) but it should not contain more than one subsequence

Regarding:

These sounds like true statements but I don't see how this is related to the code. Can you provide more context?

My goal is to get rid of the name T_A in the following lines:
https://github.com/TDAmeritrade/stumpy/blob/286cb178d201ea4378d8dde70f6d24c1b8caa5a2/stumpy/stump.py#L667-L674

And, I wanted to replace it with Q. Then, when I want to pass variables to _stump, their names would contain either Q or T [as opposed to the existing version where the variables are T_A, T_B, μ_Q, M_T, and so on]

---

If you think we should keep the name T_A [and avoid using Q], then I can keep the names and just fix the order of arguments in the [private] functions. For instance, pay attention to the order of arguments in _stump in this PR.

[seanlaw]

I can keep the names and just fix the order of arguments in the [private] functions.

Yes, let's fix this

[NimaSarajpoor]

Quick review

(1) stump, stumped, and scrumped are modified to reflect a better order of arguments in the private functions

(2) The non-normalized version of the modules mentioned in (1) are checked and they are good.

(3) Also, gpu_stump is modified so that the order of arguments in its private functions becomes consistent with the module stump. The unit test tests/test_gpu_stump.py is checked with Colab. Its tests are passing.

(4) @seanlaw: When you get a chance, please take a look at gpu_stump. If the changes are okay with you, I will apply similar changes to gpu_aamp

(5) I have avoided touching the module mstump since it is complicated, and, also, at some point I realized that it might be a good idea to leave it as is. For instance, see the following lines [from mstump module]:

https://github.com/TDAmeritrade/stumpy/blob/286cb178d201ea4378d8dde70f6d24c1b8caa5a2/stumpy/mstump.py#L514-L525

Here, the query is T_B[:, query_idx : query_idx + m], so I think the name μ_Q that is associated with T_B here may not be bad idea. So, that is why I did not change anything in mstump.

[seanlaw]

@NimaSarajpoor Looks good. Please see the one requested change

[NimaSarajpoor]

[update]
I revised gpu_aamp.py according to the changes applied to gpu_stump.py. Its tests are passing (in Colab).

@seanlaw
I did a quick scan over the modules and I think we are good.

[seanlaw]

@NimaSarajpoor Thanks again for doing this! My head is feeling much clearer now :)

[NimaSarajpoor]

Same here :)