Skip to content

Commit ddcf062

Browse files
NimaSarajpoorNimaSarajpoor
authored
Fixed #982 Ostinato Overwrites Original Time Series (#983)
* Add test to expose the overwriting-input issue * fix overwriting-input issue * minor change to improve readability * Avoid shallow copy * Add test for extracting second consensus * Insert nan values to input to better test the desired behaviour * Add test to check original input not being overwritten * Add new test to check a call to ostinato does not tamper with its input * minor change * minor change to clean code * Removed seed from test function * Fix typos * Add test to expose issue * Prevent functions from overwriting its input * Fix minor issue * Add comment * Add test functions to ostinatoed to ensure original input is unchanged * Add comment * Add comment * Add test for gpu_ostinato to ensure original input is unchanged * Add/Revise comments * Use a more explicit approach when copying original data * Explicity copy input data in non-normalized ostinato functions * Removed outdated comments * Fixed NumbaPerformanceWarning * Add tests to gpu_aamp_ostinato * Fix comments * minor fix * Revised the order of test functions * Remove unnecessary comment * simplify code by considering only one np.nan in each time series * add missing test functions to aamp_ostinato * fix format
1 parent 6971995 commit ddcf062

8 files changed

Lines changed: 391 additions & 26 deletions

‎stumpy/aamp_ostinato.py‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -305,23 +305,24 @@ def aamp_ostinato(Ts, m, p=2.0):
305305
if not isinstance(Ts, list): # pragma: no cover
306306
raise ValueError(f"`Ts` is of type `{type(Ts)}` but a `list` is expected")
307307

308+
Ts_copy = [None] * len(Ts)
308309
Ts_subseq_isfinite = [None] * len(Ts)
309310
for i, T in enumerate(Ts):
310311
(
311-
Ts[i],
312+
Ts_copy[i],
312313
Ts_subseq_isfinite[i],
313-
) = core.preprocess_non_normalized(T, m)
314+
) = core.preprocess_non_normalized(T, m, copy=True)
314315

315316
bsf_radius, bsf_Ts_idx, bsf_subseq_idx = _aamp_ostinato(
316-
Ts, m, Ts_subseq_isfinite, p
317+
Ts_copy, m, Ts_subseq_isfinite, p
317318
)
318319

319320
(
320321
central_radius,
321322
central_Ts_idx,
322323
central_subseq_idx,
323324
) = _get_aamp_central_motif(
324-
Ts, bsf_radius, bsf_Ts_idx, bsf_subseq_idx, m, Ts_subseq_isfinite, p
325+
Ts_copy, bsf_radius, bsf_Ts_idx, bsf_subseq_idx, m, Ts_subseq_isfinite, p
325326
)
326327

327328
return central_radius, central_Ts_idx, central_subseq_idx
@@ -389,15 +390,16 @@ def aamp_ostinatoed(client, Ts, m, p=2.0):
389390
if not isinstance(Ts, list): # pragma: no cover
390391
raise ValueError(f"`Ts` is of type `{type(Ts)}` but a `list` is expected")
391392

393+
Ts_copy = [None] * len(Ts)
392394
Ts_subseq_isfinite = [None] * len(Ts)
393395
for i, T in enumerate(Ts):
394396
(
395-
Ts[i],
397+
Ts_copy[i],
396398
Ts_subseq_isfinite[i],
397-
) = core.preprocess_non_normalized(T, m)
399+
) = core.preprocess_non_normalized(T, m, copy=True)
398400

399401
bsf_radius, bsf_Ts_idx, bsf_subseq_idx = _aamp_ostinato(
400-
Ts,
402+
Ts_copy,
401403
m,
402404
Ts_subseq_isfinite,
403405
p=p,
@@ -410,7 +412,7 @@ def aamp_ostinatoed(client, Ts, m, p=2.0):
410412
central_Ts_idx,
411413
central_subseq_idx,
412414
) = _get_aamp_central_motif(
413-
Ts,
415+
Ts_copy,
414416
bsf_radius,
415417
bsf_Ts_idx,
416418
bsf_subseq_idx,

‎stumpy/gpu_aamp_ostinato.py‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,16 @@ def gpu_aamp_ostinato(Ts, m, device_id=0, p=2.0):
6969
if not isinstance(Ts, list): # pragma: no cover
7070
raise ValueError(f"`Ts` is of type `{type(Ts)}` but a `list` is expected")
7171

72+
Ts_copy = [None] * len(Ts)
7273
Ts_subseq_isfinite = [None] * len(Ts)
7374
for i, T in enumerate(Ts):
7475
(
75-
Ts[i],
76+
Ts_copy[i],
7677
Ts_subseq_isfinite[i],
77-
) = core.preprocess_non_normalized(T, m)
78+
) = core.preprocess_non_normalized(T, m, copy=True)
7879

7980
bsf_radius, bsf_Ts_idx, bsf_subseq_idx = _aamp_ostinato(
80-
Ts,
81+
Ts_copy,
8182
m,
8283
Ts_subseq_isfinite,
8384
p=p,
@@ -90,7 +91,7 @@ def gpu_aamp_ostinato(Ts, m, device_id=0, p=2.0):
9091
central_Ts_idx,
9192
central_subseq_idx,
9293
) = _get_aamp_central_motif(
93-
Ts,
94+
Ts_copy,
9495
bsf_radius,
9596
bsf_Ts_idx,
9697
bsf_subseq_idx,

‎stumpy/gpu_ostinato.py‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,23 +108,37 @@ def gpu_ostinato(Ts, m, device_id=0, normalize=True, p=2.0, Ts_subseq_isconstant
108108
if Ts_subseq_isconstant is None:
109109
Ts_subseq_isconstant = [None] * len(Ts)
110110

111+
Ts_copy = [None] * len(Ts)
111112
M_Ts = [None] * len(Ts)
112113
Σ_Ts = [None] * len(Ts)
113114
for i, T in enumerate(Ts):
114-
Ts[i], M_Ts[i], Σ_Ts[i], Ts_subseq_isconstant[i] = core.preprocess(
115-
T, m, T_subseq_isconstant=Ts_subseq_isconstant[i]
115+
Ts_copy[i], M_Ts[i], Σ_Ts[i], Ts_subseq_isconstant[i] = core.preprocess(
116+
T, m, copy=True, T_subseq_isconstant=Ts_subseq_isconstant[i]
116117
)
117118

118119
bsf_radius, bsf_Ts_idx, bsf_subseq_idx = _ostinato(
119-
Ts, m, M_Ts, Σ_Ts, Ts_subseq_isconstant, device_id=device_id, mp_func=gpu_stump
120+
Ts_copy,
121+
m,
122+
M_Ts,
123+
Σ_Ts,
124+
Ts_subseq_isconstant,
125+
device_id=device_id,
126+
mp_func=gpu_stump,
120127
)
121128

122129
(
123130
central_radius,
124131
central_Ts_idx,
125132
central_subseq_idx,
126133
) = _get_central_motif(
127-
Ts, bsf_radius, bsf_Ts_idx, bsf_subseq_idx, m, M_Ts, Σ_Ts, Ts_subseq_isconstant
134+
Ts_copy,
135+
bsf_radius,
136+
bsf_Ts_idx,
137+
bsf_subseq_idx,
138+
m,
139+
M_Ts,
140+
Σ_Ts,
141+
Ts_subseq_isconstant,
128142
)
129143

130144
return central_radius, central_Ts_idx, central_subseq_idx

‎stumpy/ostinato.py‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -365,25 +365,34 @@ def ostinato(Ts, m, normalize=True, p=2.0, Ts_subseq_isconstant=None):
365365
if not isinstance(Ts, list): # pragma: no cover
366366
raise ValueError(f"`Ts` is of type `{type(Ts)}` but a `list` is expected")
367367

368+
Ts_copy = [None] * len(Ts)
368369
M_Ts = [None] * len(Ts)
369370
Σ_Ts = [None] * len(Ts)
370371
if Ts_subseq_isconstant is None:
371372
Ts_subseq_isconstant = [None] * len(Ts)
373+
372374
for i, T in enumerate(Ts):
373-
Ts[i], M_Ts[i], Σ_Ts[i], Ts_subseq_isconstant[i] = core.preprocess(
374-
T, m, T_subseq_isconstant=Ts_subseq_isconstant[i]
375+
Ts_copy[i], M_Ts[i], Σ_Ts[i], Ts_subseq_isconstant[i] = core.preprocess(
376+
T, m, copy=True, T_subseq_isconstant=Ts_subseq_isconstant[i]
375377
)
376378

377379
bsf_radius, bsf_Ts_idx, bsf_subseq_idx = _ostinato(
378-
Ts, m, M_Ts, Σ_Ts, Ts_subseq_isconstant
380+
Ts_copy, m, M_Ts, Σ_Ts, Ts_subseq_isconstant
379381
)
380382

381383
(
382384
central_radius,
383385
central_Ts_idx,
384386
central_subseq_idx,
385387
) = _get_central_motif(
386-
Ts, bsf_radius, bsf_Ts_idx, bsf_subseq_idx, m, M_Ts, Σ_Ts, Ts_subseq_isconstant
388+
Ts_copy,
389+
bsf_radius,
390+
bsf_Ts_idx,
391+
bsf_subseq_idx,
392+
m,
393+
M_Ts,
394+
Σ_Ts,
395+
Ts_subseq_isconstant,
387396
)
388397

389398
return central_radius, central_Ts_idx, central_subseq_idx
@@ -485,18 +494,19 @@ def ostinatoed(client, Ts, m, normalize=True, p=2.0, Ts_subseq_isconstant=None):
485494
if not isinstance(Ts, list): # pragma: no cover
486495
raise ValueError(f"`Ts` is of type `{type(Ts)}` but a `list` is expected")
487496

497+
Ts_copy = [None] * len(Ts)
488498
M_Ts = [None] * len(Ts)
489499
Σ_Ts = [None] * len(Ts)
490500

491501
if Ts_subseq_isconstant is None:
492502
Ts_subseq_isconstant = [None] * len(Ts)
493503
for i, T in enumerate(Ts):
494-
Ts[i], M_Ts[i], Σ_Ts[i], Ts_subseq_isconstant[i] = core.preprocess(
495-
T, m, T_subseq_isconstant=Ts_subseq_isconstant[i]
504+
Ts_copy[i], M_Ts[i], Σ_Ts[i], Ts_subseq_isconstant[i] = core.preprocess(
505+
T, m, copy=True, T_subseq_isconstant=Ts_subseq_isconstant[i]
496506
)
497507

498508
bsf_radius, bsf_Ts_idx, bsf_subseq_idx = _ostinato(
499-
Ts,
509+
Ts_copy,
500510
m,
501511
M_Ts,
502512
Σ_Ts,
@@ -510,7 +520,14 @@ def ostinatoed(client, Ts, m, normalize=True, p=2.0, Ts_subseq_isconstant=None):
510520
central_Ts_idx,
511521
central_subseq_idx,
512522
) = _get_central_motif(
513-
Ts, bsf_radius, bsf_Ts_idx, bsf_subseq_idx, m, M_Ts, Σ_Ts, Ts_subseq_isconstant
523+
Ts_copy,
524+
bsf_radius,
525+
bsf_Ts_idx,
526+
bsf_subseq_idx,
527+
m,
528+
M_Ts,
529+
Σ_Ts,
530+
Ts_subseq_isconstant,
514531
)
515532

516533
return central_radius, central_Ts_idx, central_subseq_idx

‎tests/test_aamp_ostinato.py‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,117 @@ def test_deterministic_ostinatoed(seed, dask_cluster):
8585
npt.assert_almost_equal(ref_radius, comp_radius)
8686
npt.assert_almost_equal(ref_Ts_idx, comp_Ts_idx)
8787
npt.assert_almost_equal(ref_subseq_idx, comp_subseq_idx)
88+
89+
90+
def test_input_not_overwritten_ostinato():
91+
# aamp_ostinato preprocesses its input, a list of time series,
92+
# by replacing nan value with 0 in each time series.
93+
# This test ensures that the original input is not overwritten
94+
m = 50
95+
Ts = [np.random.rand(n) for n in [64, 128, 256]]
96+
for T in Ts:
97+
T[0] = np.nan
98+
99+
# raise error if aamp_ostinato overwrite its input
100+
Ts_input = [T.copy() for T in Ts]
101+
stumpy.aamp_ostinato(Ts_input, m)
102+
for i in range(len(Ts)):
103+
T_ref = Ts[i]
104+
T_comp = Ts_input[i]
105+
npt.assert_almost_equal(T_ref[np.isfinite(T_ref)], T_comp[np.isfinite(T_comp)])
106+
107+
108+
def test_extract_several_consensus_ostinato():
109+
# This test is to further ensure that the function `aamp_ostinato`
110+
# does not tamper with the original data.
111+
Ts = [np.random.rand(n) for n in [256, 512, 1024]]
112+
Ts_ref = [T.copy() for T in Ts]
113+
Ts_comp = [T.copy() for T in Ts]
114+
115+
m = 20
116+
117+
k = 5 # Get the first `k` consensus motifs
118+
for _ in range(k):
119+
# Find consensus motif and its NN in each time series in Ts_comp
120+
# Remove them from Ts_comp as well as Ts_ref, and assert that the
121+
# two time series are the same
122+
radius, Ts_idx, subseq_idx = stumpy.aamp_ostinato(Ts_comp, m)
123+
consensus_motif = Ts_comp[Ts_idx][subseq_idx : subseq_idx + m].copy()
124+
for i in range(len(Ts_comp)):
125+
if i == Ts_idx:
126+
query_idx = subseq_idx
127+
else:
128+
query_idx = None
129+
130+
idx = np.argmin(
131+
stumpy.core.mass(
132+
consensus_motif, Ts_comp[i], normalize=False, query_idx=query_idx
133+
)
134+
)
135+
Ts_comp[i][idx : idx + m] = np.nan
136+
Ts_ref[i][idx : idx + m] = np.nan
137+
138+
npt.assert_almost_equal(
139+
Ts_ref[i][np.isfinite(Ts_ref[i])], Ts_comp[i][np.isfinite(Ts_comp[i])]
140+
)
141+
142+
143+
def test_input_not_overwritten_ostinatoed(dask_cluster):
144+
# ostinatoed preprocesses its input, a list of time series,
145+
# by replacing nan value with 0 in each time series.
146+
# This test ensures that the original input is not overwritten
147+
with Client(dask_cluster) as dask_client:
148+
m = 50
149+
Ts = [np.random.rand(n) for n in [64, 128, 256]]
150+
for T in Ts:
151+
T[0] = np.nan
152+
153+
# raise error if ostinato overwrite its input
154+
Ts_input = [T.copy() for T in Ts]
155+
stumpy.aamp_ostinatoed(dask_client, Ts_input, m)
156+
for i in range(len(Ts)):
157+
T_ref = Ts[i]
158+
T_comp = Ts_input[i]
159+
npt.assert_almost_equal(
160+
T_ref[np.isfinite(T_ref)], T_comp[np.isfinite(T_comp)]
161+
)
162+
163+
164+
def test_extract_several_consensus_ostinatoed(dask_cluster):
165+
# This test is to further ensure that the function `ostinatoed`
166+
# does not tamper with the original data.
167+
Ts = [np.random.rand(n) for n in [256, 512, 1024]]
168+
Ts_ref = [T.copy() for T in Ts]
169+
Ts_comp = [T.copy() for T in Ts]
170+
171+
m = 20
172+
173+
with Client(dask_cluster) as dask_client:
174+
k = 5 # Get the first `k` consensus motifs
175+
for _ in range(k):
176+
# Find consensus motif and its NN in each time series in Ts_comp
177+
# Remove them from Ts_comp as well as Ts_ref, and assert that the
178+
# two time series are the same
179+
radius, Ts_idx, subseq_idx = stumpy.aamp_ostinatoed(dask_client, Ts_comp, m)
180+
consensus_motif = Ts_comp[Ts_idx][subseq_idx : subseq_idx + m].copy()
181+
for i in range(len(Ts_comp)):
182+
if i == Ts_idx:
183+
query_idx = subseq_idx
184+
else:
185+
query_idx = None
186+
187+
idx = np.argmin(
188+
stumpy.core.mass(
189+
consensus_motif,
190+
Ts_comp[i],
191+
normalize=False,
192+
query_idx=query_idx,
193+
)
194+
)
195+
Ts_comp[i][idx : idx + m] = np.nan
196+
Ts_ref[i][idx : idx + m] = np.nan
197+
198+
npt.assert_almost_equal(
199+
Ts_ref[i][np.isfinite(Ts_ref[i])],
200+
Ts_comp[i][np.isfinite(Ts_comp[i])],
201+
)

‎tests/test_gpu_aamp_ostinato.py‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import naive
1313
import pytest
1414

15-
from stumpy import gpu_aamp_ostinato
15+
from stumpy import core, gpu_aamp_ostinato
1616

1717
TEST_THREADS_PER_BLOCK = 10
1818

@@ -53,3 +53,60 @@ def test_deterministic_gpu_aamp_ostinato(seed):
5353
npt.assert_almost_equal(ref_radius, comp_radius)
5454
npt.assert_almost_equal(ref_Ts_idx, comp_Ts_idx)
5555
npt.assert_almost_equal(ref_subseq_idx, comp_subseq_idx)
56+
57+
58+
@pytest.mark.filterwarnings("ignore", category=NumbaPerformanceWarning)
59+
@patch("stumpy.config.STUMPY_THREADS_PER_BLOCK", TEST_THREADS_PER_BLOCK)
60+
def test_input_not_overwritten():
61+
# gpu_aamp_ostinato preprocesses its input, a list of time series,
62+
# by replacing nan value with 0 in each time series.
63+
# This test ensures that the original input is not overwritten
64+
m = 50
65+
Ts = [np.random.rand(n) for n in [64, 128, 256]]
66+
for T in Ts:
67+
T[0] = np.nan
68+
69+
# raise error if gpu_aamp_ostinato overwrite its input
70+
Ts_input = [T.copy() for T in Ts]
71+
gpu_aamp_ostinato(Ts_input, m)
72+
for i in range(len(Ts)):
73+
T_ref = Ts[i]
74+
T_comp = Ts_input[i]
75+
npt.assert_almost_equal(T_ref[np.isfinite(T_ref)], T_comp[np.isfinite(T_comp)])
76+
77+
78+
@pytest.mark.filterwarnings("ignore", category=NumbaPerformanceWarning)
79+
@patch("stumpy.config.STUMPY_THREADS_PER_BLOCK", TEST_THREADS_PER_BLOCK)
80+
def test_extract_several_consensus():
81+
# This test is to further ensure that the function `gpu_aamp_ostinato`
82+
# does not tamper with the original data.
83+
Ts = [np.random.rand(n) for n in [256, 512, 1024]]
84+
Ts_ref = [T.copy() for T in Ts]
85+
Ts_comp = [T.copy() for T in Ts]
86+
87+
m = 20
88+
89+
k = 5 # Get the first `k` consensus motifs
90+
for _ in range(k):
91+
# Find consensus motif and its NN in each time series in Ts_comp
92+
# Remove them from Ts_comp as well as Ts_ref, and assert that the
93+
# two time series are the same
94+
radius, Ts_idx, subseq_idx = gpu_aamp_ostinato(Ts_comp, m)
95+
consensus_motif = Ts_comp[Ts_idx][subseq_idx : subseq_idx + m].copy()
96+
for i in range(len(Ts_comp)):
97+
if i == Ts_idx:
98+
query_idx = subseq_idx
99+
else:
100+
query_idx = None
101+
102+
idx = np.argmin(
103+
core.mass(
104+
consensus_motif, Ts_comp[i], normalize=False, query_idx=query_idx
105+
)
106+
)
107+
Ts_comp[i][idx : idx + m] = np.nan
108+
Ts_ref[i][idx : idx + m] = np.nan
109+
110+
npt.assert_almost_equal(
111+
Ts_ref[i][np.isfinite(Ts_ref[i])], Ts_comp[i][np.isfinite(Ts_comp[i])]
112+
)

0 commit comments

Comments
 (0)