Improve MASS precision for near-perfect correlations - #1174
Improve MASS precision for near-perfect correlations#1174jackwalkerlabs wants to merge 1 commit into
Conversation
Signed-off-by: Jack Walker <jackwalkerlabs@gmail.com>
|
Review these changes at https://app.gitnotebooks.com/stumpy-dev/stumpy/pull/1174 |
seanlaw
left a comment
There was a problem hiding this comment.
@jackwalkerlabs Please see the following requested changes
To preserve backwards compatibility, I don't think we should change calculate_distance_profile. Instead, I we should create a new function inside of stumpy/core.py that is called _mass_postprocess that simply checks the output of mass (normalized) and recomputes only the distances that are below the described threshold.
In addition to the tests that you've added, can you please:
- Pull the latest commits from the
mainbranch into this development branch - Add the following test to
tests/test_precision
import test_motifs.naive_motifs
from stumpy import motifs
def test_motifs():
state = mersenne.seed_to_state("test_precision::test_motifs")
with rng.fix_state(state):
T = rng.RNG.rand(64)
m = 3
max_motifs = 3
max_matches = 4
max_distance = np.inf
cutoff = np.inf
# naive
# `max_distance` and `cutoff` are hard-coded, and set to np.inf.
ref_distances, ref_indices = naive_motifs(T, m, max_motifs, max_matches)
# performant
mp = naive.stump(T, m, row_wise=True)
comp_distance, comp_indices = motifs(
T,
mp[:, 0].astype(np.float64),
min_neighbors=1,
max_distance=max_distance,
cutoff=cutoff,
max_matches=max_matches,
max_motifs=max_motifs,
)
npt.assert_almost_equal(ref_indices, comp_indices)
> npt.assert_almost_equal(ref_distances, comp_distance)
If your fix is correct, then this test should pass
| ref = naive.distance_profile(Q, T, m) | ||
| comp = core.mass(Q, T) | ||
|
|
||
| npt.assert_allclose(ref, comp, rtol=1e-12, atol=1e-12) |
There was a problem hiding this comment.
What happens to the test when we use the default npt.assert_allclose parameters?
| ref = naive.distance_profile(Q, T, m) | ||
| comp = core.mass(Q, T) | ||
|
|
||
| npt.assert_allclose(ref, comp, rtol=1e-12, atol=1e-12) |
There was a problem hiding this comment.
What happens to the test when we use the default npt.assert_allclose parameters?
Summary
Fixes #1171.
MASS computes squared z-normalized distance as
2 * m * (1 - rho). For the nearly affine-equivalent length-3 subsequences in the reported seed,1 - rhois about1.46e-13, so cancellation in the sliding correlation formula amplifies ordinary float64 rounding. The motif indices are correct, but the resulting distance differs materially from a direct calculation.This keeps the existing FFT/correlation path for ordinary candidates. When its squared distance is below
2 * m * sqrt(float64 epsilon)—including values that incorrectly round to zero—_massrecomputes only that candidate from pointwise differences between the two z-normalized subsequences. Constant and non-finite handling stays on the existing path.Two deterministic regressions cover:
3.7e-7Both regressions fail on clean
mainand pass with this change. The originalSTUMPY_SEED=462698499motif test also passes with JIT disabled and enabled without weakening its assertions.Performance
For
m=100over 99,901 candidates with one threshold candidate, the compiled profile kernel measured a median 105.5 microseconds on the existing path and 132.5 microseconds with the stable fallback, a 27-microsecond delta. End-to-endmasstiming remained within run-to-run noise in the same environment (8.058 ms on cleanmain, 7.951 ms with this change). The extra work isO(c * m), wherecis the number of extremely close candidates.Validation
Using the dependency versions from #1171 on Python 3.13.3:
./setup.sh dev && ./test.shpasses both the compiled and coverage phases; CUDA simulation passes and coverage is 100% across 14,936 statementsgit diff --checkchecks passAI assistance was used for more than 15% of this change. I reviewed the numerical formulation, verified it against 100-digit Decimal references, mutation-checked both regressions against clean
main, and ran the validation above.Pull Request Checklist
Below is a simple checklist but please do not hesitate to ask for assistance!
black(i.e.,python -m pip install blackorconda install -c conda-forge black)flake8(i.e.,python -m pip install flake8orconda install -c conda-forge flake8)pytest-cov(i.e.,python -m pip install pytest-covorconda install -c conda-forge pytest-cov)black --exclude=".*\.ipynb" --extend-exclude=".venv" --diff ./in the root stumpy directoryflake8 --extend-exclude=.venv ./in the root stumpy directory./setup.sh dev && ./test.shin the root stumpy directory and ensured that all tests are passing locallyPlease do not commit any code to avoid/circumvent a failing test and, instead, engage in a discussion (below) to determine the best course of action.
Only request a review after the checklist above is fully completed!