Repository navigation
Conversation
|
Review these changes at https://app.gitnotebooks.com/stumpy-dev/stumpy/pull/1220 |
Contributor
|
@karthiksenv Thanks for the PR |
16 of 18 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to #1175, following Sean's request to migrate the remaining comparisons one test file per PR.
tests/test_non_normalized_decorator.pynow usesassert_allclosefor its 43 comparisons, with computed values first,atol=1.5e-07, no explicitrtol, andcmp_/ref_names. Four comparison sites convert object-typed numeric arrays tofloat64, 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
bd866ffcompleted 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
Futuretimeout after all 102 existingtest_aamped.pycases 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!
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 locally (diagnostic reruns and coverage supplement described above)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!