-
Notifications
You must be signed in to change notification settings - Fork 341
Refactor/sdp structure cleanup #1126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,10 +12,9 @@ | |
| from numba import cuda, njit, prange | ||
| from scipy import linalg | ||
| from scipy.ndimage import maximum_filter1d, minimum_filter1d | ||
| from scipy.signal import convolve | ||
| from scipy.spatial.distance import cdist | ||
|
|
||
| from . import config | ||
| from . import config, sdp | ||
|
|
||
| try: | ||
| from numba.cuda.cudadrv.driver import _raise_driver_not_found | ||
|
|
@@ -649,36 +648,9 @@ def check_window_size(m, max_size=None, n=None): | |
| warnings.warn(msg) | ||
|
|
||
|
|
||
| @njit(fastmath=config.STUMPY_FASTMATH_TRUE) | ||
| def _sliding_dot_product(Q, T): | ||
| """ | ||
| A Numba JIT-compiled implementation of the sliding window dot product. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| Q : numpy.ndarray | ||
| Query array or subsequence | ||
|
|
||
| T : numpy.ndarray | ||
| Time series or sequence | ||
|
|
||
| Returns | ||
| ------- | ||
| out : numpy.ndarray | ||
| Sliding dot product between `Q` and `T`. | ||
| """ | ||
| m = Q.shape[0] | ||
| l = T.shape[0] - m + 1 | ||
| out = np.empty(l) | ||
| for i in range(l): | ||
| out[i] = np.dot(Q, T[i : i + m]) | ||
|
|
||
| return out | ||
|
|
||
|
|
||
| def sliding_dot_product(Q, T): | ||
| """ | ||
| Use FFT convolution to calculate the sliding window dot product. | ||
| Calculate the sliding window dot product. | ||
|
|
||
| Parameters | ||
| ---------- | ||
|
|
@@ -692,27 +664,8 @@ def sliding_dot_product(Q, T): | |
| ------- | ||
| output : numpy.ndarray | ||
| Sliding dot product between `Q` and `T`. | ||
|
|
||
| Notes | ||
| ----- | ||
| Calculate the sliding dot product | ||
|
|
||
| `DOI: 10.1109/ICDM.2016.0179 \ | ||
| <https://www.cs.ucr.edu/~eamonn/PID4481997_extend_Matrix%20Profile_I.pdf>`__ | ||
|
|
||
| See Table I, Figure 4 | ||
|
|
||
| Following the inverse FFT, Fig. 4 states that only cells [m-1:n] | ||
| contain valid dot products | ||
|
|
||
| Padding is done automatically in fftconvolve step | ||
| """ | ||
| n = T.shape[0] | ||
| m = Q.shape[0] | ||
| Qr = np.flipud(Q) # Reverse/flip Q | ||
| QT = convolve(Qr, T) | ||
|
|
||
| return QT.real[m - 1 : n] | ||
| return sdp._sliding_dot_product(Q, T) | ||
|
|
||
|
|
||
| @njit( | ||
|
|
@@ -1327,7 +1280,7 @@ def _p_norm_distance_profile(Q, T, p=2.0): | |
| T_squared[i] = ( | ||
| T_squared[i - 1] - T[i - 1] * T[i - 1] + T[i + m - 1] * T[i + m - 1] | ||
| ) | ||
| QT = _sliding_dot_product(Q, T) | ||
| QT = sdp._njit_sliding_dot_product(Q, T) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the name being While
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
YES, and YES!!! |
||
| for i in range(l): | ||
| p_norm_profile[i] = Q_squared + T_squared[i] - 2.0 * QT[i] | ||
| else: | ||
|
|
@@ -1900,7 +1853,7 @@ def _mass_distance_matrix( | |
| if np.any(~np.isfinite(Q[i : i + m])): # pragma: no cover | ||
| distance_matrix[i, :] = np.inf | ||
| else: | ||
| QT = _sliding_dot_product(Q[i : i + m], T) | ||
| QT = sdp._njit_sliding_dot_product(Q[i : i + m], T) | ||
| distance_matrix[i, :] = _mass( | ||
| Q[i : i + m], | ||
| T, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import numpy as np | ||
| from numba import njit | ||
|
|
||
| from . import config | ||
|
|
||
|
|
||
| @njit(fastmath=config.STUMPY_FASTMATH_TRUE) | ||
| def _njit_sliding_dot_product(Q, T): | ||
| """ | ||
| A Numba JIT-compiled implementation of the sliding window dot product. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| Q : numpy.ndarray | ||
| Query array or subsequence | ||
|
|
||
| T : numpy.ndarray | ||
| Time series or sequence | ||
|
|
||
| Returns | ||
| ------- | ||
| out : numpy.ndarray | ||
| Sliding dot product between `Q` and `T`. | ||
| """ | ||
| m = Q.shape[0] | ||
| l = T.shape[0] - m + 1 | ||
| out = np.empty(l) | ||
| for i in range(l): | ||
| out[i] = np.dot(Q, T[i : i + m]) | ||
|
|
||
| return out | ||
|
|
||
|
|
||
| def _sliding_dot_product(Q, T): | ||
| """ | ||
| A wrapper function for the Numba JIT-compiled implementation of the sliding | ||
| window dot product. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| Q : numpy.ndarray | ||
| Query array or subsequence | ||
|
|
||
| T : numpy.ndarray | ||
| Time series or sequence | ||
|
|
||
| Returns | ||
| ------- | ||
| out : numpy.ndarray | ||
| Sliding dot product between `Q` and `T`. | ||
| """ | ||
| return _njit_sliding_dot_product(Q, T) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import numpy as np | ||
| import pytest | ||
| from numpy import testing as npt | ||
|
|
||
| from stumpy import sdp | ||
|
|
||
|
|
||
| def naive_rolling_window_dot_product(Q, T): | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have a similar function in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only reason why I didn't want to put it in
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Noted. Since I am now using it in |
||
| window = len(Q) | ||
| result = np.zeros(len(T) - window + 1) | ||
| for i in range(len(result)): | ||
| result[i] = np.dot(T[i : i + window], Q) | ||
| return result | ||
|
|
||
|
|
||
| test_data = [ | ||
| (np.array([-1, 1, 2], dtype=np.float64), np.array(range(5), dtype=np.float64)), | ||
| ( | ||
| np.array([9, 8100, -60], dtype=np.float64), | ||
| np.array([584, -11, 23, 79, 1001], dtype=np.float64), | ||
| ), | ||
| (np.random.uniform(-1000, 1000, [8]), np.random.uniform(-1000, 1000, [64])), | ||
| ] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("Q, T", test_data) | ||
| def test_njit_sliding_dot_product(Q, T): | ||
| ref_mp = naive_rolling_window_dot_product(Q, T) | ||
| comp_mp = sdp._njit_sliding_dot_product(Q, T) | ||
| npt.assert_almost_equal(ref_mp, comp_mp) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("Q, T", test_data) | ||
| def test_sliding_dot_product(Q, T): | ||
| ref_mp = naive_rolling_window_dot_product(Q, T) | ||
| comp_mp = sdp._sliding_dot_product(Q, T) | ||
| npt.assert_almost_equal(ref_mp, comp_mp) | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seanlaw
Regarding
core.sliding_dot_prodiuct: Is this what you meant when you said in this comment (see notes on PR 1):?
I think we should handle
sdp._convolve_sliding_dot_product(see PR 2 in this comment) in this PR. The functioncore.sliding_dot_productin the branchmaincan be copied to thesdpmodule, and renamed to_convolve_sliding_dot_product.sdp._sliding_dot_productshould point tosdp._convolve_sliding_dot_product. This is not a serious issue if the plan is to handle PR 2 quickly right after PR 1.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that makes perfect sense. It's not actually new code or new SDP methods. It's simply a refactoring of existing code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And then, in subsequent PRs, it should point to multiple functions using some branching logic, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct 👍