passing-input-with-finite-value to `core._mass` is being violated

[NimaSarajpoor]

The function core.mass(Q, T, ...) calls core.preprocess(T, m, ...), which return the following values:

• T
• M_T
• Σ_T
• T_subseq_isconstant

We then pass these values to core._mass. However, these M_T and Σ_T can contain non-finite value if the original T has non-finite value. In fact, if you just follow the function core._mass and go the callee function and then continue till you reach the very end of the road, you can see that at the end, we use that in the function core._calculate_squared_distance to determine whether a distance should be infinite or not.

https://github.com/TDAmeritrade/stumpy/blob/3077d0ddfb315464321dc86f8ec3bf2cab9ce3b1/stumpy/core.py#L1093-L1094

In the function core._calculate_squared_distance, the flag is correctly set. Was wondering if should do the same for core._mass?

[seanlaw]

What exactly are you proposing? To set fastmath={"nsz", "arcp", "contract", "afn", "reassoc"} for core._mass? Maybe... I don't know.

[NimaSarajpoor]

Right!

I am now thinking what if we just avoid setting fastmath here for core._mass because this function does not do any arithmetic operations. I can try it out and check its impact on performance if there is any.

[NimaSarajpoor]

In the following, A --> B means function A calls function B.

# in stumpy.core
mass --> _mass
_mass --> calculate_distance_profile
calculate_distance_profile -->  _calculate_squared_distance_profile
_calculate_squared_distance_profile --> _calculate_squared_distance

I removed the fastmath flag for all expect the last one, which comes with fastmath={"nsz", "arcp", "contract", "afn", "reassoc"}.

I then compared the performance of core.mass for the following input:

seed = 0
np.random.seed(seed)
Q = np.random.rand(100)
T = np.random.rand(1_000_000)

I ran the following script:

import numpy as np
import time
import stumpy

def check_mass_performance():
    n_iter = 100

    seed = 0
    np.random.seed(seed)
    Q = np.random.rand(100)
    T = np.random.rand(1000000)

    t_lst = []
    for _ in range(n_iter):
        start = time.time()
        stumpy.mass(Q, T)
        t_lst.append(time.time() - start)
    
    return np.mean(t_lst[1:]), np.std(t_lst[1:])


if __name__ == "__main__":
    out = check_mass_performance()
    print(f'mean: {out[0]}, std: {out[1]}' )

And I ran it for several times.

Running time	mean	std
case 1	0.096-0.097	~0.002
case 2	0.096-0.097	~0.002

where case 1 is the existing version and case 2 is when the fastmath flags are removed except for core. _calculate_squared_distance. Although I did not do any statistical test, it seems that there is no certain gain in having fastmath flags on all those njit functions.

[seanlaw]

I am now thinking what if we just avoid setting fastmath here for core._mass because this function does not do any arithmetic operations. I can try it out and check its impact on performance if there is any.

I see. When there is no speed gain, I would rather be explicit rather than implicit. In this case, it appears that the inputs/outputs can all contain non-finite values. This means that fastmath=True is "wrong". I would simply make every level of the nested function calls fastmath={"nsz", "arcp", "contract", "afn", "reassoc"}. That would be my preference as this would be very explicit and one would never have to question whether each function was allowed to have non-finite values (the answer is "yes!").

I think this should be handled together with #708 though

[NimaSarajpoor]

This means that fastmath=True is "wrong". I would simply make every level of the nested function calls fastmath={"nsz", "arcp", "contract", "afn", "reassoc"}. That would be my preference as this would be very explicit

That's a good idea! Got it!!

I think this should be handled together with #708 though

Okay. So, the plan is to first solve #708 (and this issue, i.e. #1011), and then resume the work on #1012 as suggested in #1012 (comment).

[seanlaw]

Yes, thank you @NimaSarajpoor!

[seanlaw]

@NimaSarajpoor I believe that this issue is resolved in #1025 and can be closed as completed? What do you think?

[NimaSarajpoor]

@seanlaw
Correct. Thanks for pointing that out! Going to close now.