Skip to content

Migrate tests/test_mmotifs.py to NumPy array comparisons - #1212

Open
karthiksenv wants to merge 2 commits into
stumpy-dev:mainfrom
karthiksenv:codex/migrate-mmotifs-comparisons
Open

karthiksenv wants to merge 2 commits into
stumpy-dev:mainfrom
karthiksenv:codex/migrate-mmotifs-comparisons

Conversation

@karthiksenv

@karthiksenv karthiksenv commented Nov 5, 2026 •

Copy link
Copy Markdown

Related to #1175. This covers tests/test_mmotifs.py, as agreed in the issue.

The seven multidimensional motif tests use assert_allclose for all 28 comparisons, with atol=1.5e-07 and no explicit rtol. Each comparison puts the computed value first and uses the cmp_ and ref_ prefixes from the other migrations.

The two-motif fixture previously expected a self-match distance of 1.68587394e-07. It now expects zero: mmotifs passes the motif index through match to core.mass, which explicitly sets the query's self-match distance to zero. The tighter tolerance exposed this stale expectation. Other reference values are unchanged.

After the review corrections, all seven changed tests passed on Windows with Numba JIT enabled and disabled, using Python 3.12.15, NumPy 2.5.4, Numba 0.68.1 and seed 1175. Each run reported the existing where warning from core.z_norm. Black and flake8 passed for the changed file.

The same seven tests also passed on Ubuntu with JIT enabled and disabled after these corrections, with the same existing warnings.

Before the review corrections, the full Ubuntu development setup and test script ran on commit 3501ed5 with seed 1175. The JIT phase had 2,363 passes and 40 skips, and the coverage phase had 2,510 passes and 17 skips. The initial coverage report missed the default-seed initialization line because a seed was supplied. Running the existing two RNG tests with STUMPY_SEED unset and rerunning the project's report produced 100% coverage across 15,105 statements. Optional Ray and pyFFTW integrations were unavailable. Repository-wide formatting, import-order, lint, docstring and package checks passed. This full run has not been repeated after the comparison and fixture corrections.

OpenAI Codex assisted with the migration, local validation and PR text.

This PR replaces #1211, which the checklist workflow closed before the template was added.

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
    • less than 25% of the code in this pull request
    • 25-50% of the code in this pull request
    • more than 50% 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!

@gitnotebooks

gitnotebooks Bot commented Nov 5, 2026

Copy link
Copy Markdown

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

@karthiksenv
karthiksenv marked this pull request as ready for review November 6, 2026 04:01
@karthiksenv
karthiksenv requested a review from seanlaw as a code owner November 6, 2026 04:01
@seanlaw

seanlaw commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

@karthiksenv I see a lot of inconsistencies in the PR. Please cross reference the To Do List as well as the other related
PRs

@karthiksenv

Copy link
Copy Markdown
Author

Thanks, Sean. I missed those conventions in the first version. I checked the checklist and #1181/#1202 and pushed 9bbe330: all 28 comparisons now use assert_allclose with atol=1.5e-07 and no explicit rtol, keeping the cmp_/ref_ names and computed values first.

The tighter tolerance exposed one stale fixture value. The second motif's self-match expected 1.68587394e-07, but mmotifs passes its index through match to core.mass, which explicitly sets that distance to zero. I corrected that expectation to zero; the other reference values are unchanged.

All seven tests pass on Windows and Ubuntu, with JIT enabled and disabled. Black and flake8 pass too. I updated the description to distinguish these reruns from the full-suite results on the earlier commit. Codex assisted with this follow-up and reply.

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.

2 participants