Skip to content

Migrate non-normalized decorator comparisons to assert_allclose - #1220

Merged
seanlaw merged 1 commit into
stumpy-dev:mainfrom
karthiksenv:codex/migrate-non-normalized-comparisons
Nov 14, 2026
Merged

seanlaw merged 1 commit into
stumpy-dev:mainfrom
karthiksenv:codex/migrate-non-normalized-comparisons

Conversation

@karthiksenv

@karthiksenv karthiksenv commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Related to #1175, following Sean's request to migrate the remaining comparisons one test file per PR.

tests/test_non_normalized_decorator.py now uses assert_allclose for its 43 comparisons, with computed values first, atol=1.5e-07, no explicit rtol, and cmp_/ref_ names. Four comparison sites convert object-typed numeric arrays to float64, following the existing matrix-profile tests. Fixture values, RNG call order and non-comparison logic are unchanged, as checked by an AST comparison.

The 40 CPU cases passed on Windows with JIT enabled and disabled; six GPU cases were skipped in those runs. All six GPU cases passed separately in the CUDA simulator, in two runs covering the two matrix-profile cases and the four remaining cases. Black, flake8 and the structural audit passed on the final source. The four warnings about missing motifs and window size also appeared with the original test file and matching baseline runtime in a Windows no-JIT run.

An earlier JIT run lost its Dask workers after the scheduler and nannies reported roughly 23 minutes unresponsive. It failed during communication, before the comparison assertion. A fresh-cluster rerun passed all 40 CPU cases without changing the source or timeout settings. The original failure log is preserved.

Ubuntu focused checks passed: 46 cases with JIT disabled and CUDA simulation enabled, and 40 passed with six GPU skips with JIT enabled. Full validation of bd866ff completed all 49 test files in each phase: 2,363 passed and 32 skipped with JIT enabled, then 2,510 passed and 17 skipped in the coverage phase. Setup, dependency checks, repository-wide formatting/lint, docstring and fastmath checks passed. The final report reached 100% coverage across 15,105 statements, and all 216 archived source files and executable bits matched the commit after testing. Ray and pyFFTW were unavailable; CUDA used the simulator. No physical GPU testing was performed.

The completed JIT phase logged one background Future timeout after all 102 existing test_aamped.py cases passed. That file and the runtime code are unchanged by this PR, and the group runs before the changed decorator tests. Separate reruns passed all 102 cases with JIT enabled and disabled, without repeating the diagnostic or changing source, assertions or timeouts. Its cause remains unproven, and the original log is preserved. The strict validation helper rejected that diagnostic and the initial single-line RNG coverage gap; these reruns and the final report were reviewed separately. Running the existing two RNG tests with the fixed seed unset filled the default initialization line's gap. An earlier interrupted full run is also preserved; the full counts above refer to the completed run on the same verified source.

The hosted Tests workflow passed on bd866ff: all 31 jobs succeeded across the generated Python/platform matrix.

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

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 (diagnostic reruns and coverage supplement described above)
  • 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 13, 2026

Copy link
Copy Markdown

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

@karthiksenv
karthiksenv marked this pull request as ready for review November 14, 2026 05:16
@karthiksenv
karthiksenv requested a review from seanlaw as a code owner November 14, 2026 05:16
@seanlaw
seanlaw merged commit 155ddac into stumpy-dev:main Nov 14, 2026
33 checks passed
@seanlaw

seanlaw commented Nov 14, 2026

Copy link
Copy Markdown
Contributor

@karthiksenv Thanks for the PR

@austinchennn austinchennn mentioned this pull request Nov 14, 2026
16 of 18 tasks
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