Skip to content

Improve MASS precision for near-perfect correlations - #1174

Open
jackwalkerlabs wants to merge 1 commit into
stumpy-dev:mainfrom
jackwalkerlabs:fix/motif-test-precision
Open

Improve MASS precision for near-perfect correlations#1174
jackwalkerlabs wants to merge 1 commit into
stumpy-dev:mainfrom
jackwalkerlabs:fix/motif-test-precision

Conversation

@jackwalkerlabs

Copy link
Copy Markdown

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 - rho is about 1.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—_mass recomputes 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:

  • the nearly affine subsequences from the reported failure, where the old result differs from the 100-digit reference by roughly 3.7e-7
  • a non-identical pair whose correlation result incorrectly rounds to zero

Both regressions fail on clean main and pass with this change. The original STUMPY_SEED=462698499 motif test also passes with JIT disabled and enabled without weakening its assertions.

Performance

For m=100 over 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-end mass timing remained within run-to-run noise in the same environment (8.058 ms on clean main, 7.951 ms with this change). The extra work is O(c * m), where c is the number of extremely close candidates.

Validation

Using the dependency versions from #1171 on Python 3.13.3:

  • the focused core, precision, and motif suites pass with JIT disabled: 274 passed
  • the same suites pass with JIT enabled: 272 passed, 2 expected GPU skips
  • ./setup.sh dev && ./test.sh passes both the compiled and coverage phases; CUDA simulation passes and coverage is 100% across 14,936 statements
  • Black, isort, Flake8, docstring, package-import, fastmath-policy, and git diff --check checks pass
  • Ray and pyFFTW are not installed; the repository harness reports and skips those optional paths

AI 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!

  • Read our Contributing Guide
  • Referenced a Github issue (or create one if one doesn't already exist)
  • Read and reviewed all of the comments in the Github issue that you've referenced (along with cross-referenced issues/pull requests) to ensure that the issue still requires a pull request
  • Checked that the issue has not already been assigned to anybody else or is already being addressed in another pull request
  • Left a meaningful comment on the original Github issue to discuss the detailed approach for your contribution and received confirmation from the maintainers before proceeding with this pull request
  • Forked, cloned, and checked out the newest version of the code
  • Created a new branch
  • Made necessary code changes
  • Installed black (i.e., python -m pip install black or conda install -c conda-forge black)
  • Installed flake8 (i.e., python -m pip install flake8 or conda install -c conda-forge flake8)
  • Installed pytest-cov (i.e., python -m pip install pytest-cov or conda install -c conda-forge pytest-cov)
  • Ran black --exclude=".*\.ipynb" --extend-exclude=".venv" --diff ./ in the root stumpy directory
  • Ran flake8 --extend-exclude=.venv ./ in the root stumpy directory
  • Ran ./setup.sh dev && ./test.sh in the root stumpy directory and ensured that all tests are passing locally
  • Check this box if AI code assistance was used to generate 15%+ of the code in this pull request

Please 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!

Signed-off-by: Jack Walker <jackwalkerlabs@gmail.com>
@jackwalkerlabs
jackwalkerlabs requested a review from seanlaw as a code owner August 5, 2026 00:23
@gitnotebooks

gitnotebooks Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review these changes at https://app.gitnotebooks.com/stumpy-dev/stumpy/pull/1174

@seanlaw seanlaw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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:

  1. Pull the latest commits from the main branch into this development branch
  2. 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

Comment thread tests/test_precision.py
ref = naive.distance_profile(Q, T, m)
comp = core.mass(Q, T)

npt.assert_allclose(ref, comp, rtol=1e-12, atol=1e-12)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens to the test when we use the default npt.assert_allclose parameters?

Comment thread tests/test_precision.py
ref = naive.distance_profile(Q, T, m)
comp = core.mass(Q, T)

npt.assert_allclose(ref, comp, rtol=1e-12, atol=1e-12)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens to the test when we use the default npt.assert_allclose parameters?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate Failed Test in test_motifs.py

2 participants