Replace wall-clock guards in equality tests with operation counts - #781
Merged
Conversation
The `@elapsed(...) < 0.1` assertions added for the stored-entries-only `isequal`/`==` paths are wall-clock thresholds on shared CI runners, and the matrix variants build 10^5 x 10^5 matrices (800 KB column pointers) for each of several wrapper combinations. Replace them with a deterministic check: using the existing `Counting` eltype in sparsematrix_ops.jl, assert that a comparison performs at most nnz(A) + nnz(B) element comparisons, where the generic AbstractArray fallback would perform length(A). The NaN, signed-zero and stored-zero semantics tests are kept at small sizes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LgBHUw9Hp7YW5ub29B4R5y
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #781 +/- ##
=======================================
Coverage ? 92.06%
=======================================
Files ? 12
Lines ? 8377
Branches ? 0
=======================================
Hits ? 7712
Misses ? 665
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Follow-up to #766 and #768 (and item 6 of the test-time review).
The
@elapsed(...) < 0.1assertions guarding the stored-entries-onlyisequal/==paths are wall-clock thresholds, which are the main flake risk on loaded CI runners. The matrix variants also build10^5 x 10^5matrices (an 800 KB column-pointer array each) for seven wrapper combinations.They are replaced with a deterministic check.
sparsematrix_ops.jlalready has aCountingeltype whose==increments a global counter; this PR gives itisequaltoo and asserts that comparing two sparse arrays performs at mostnnz(A) + nnz(B)element comparisons. The genericAbstractArrayfallback would performlength(A)of them, so a regression to it fails the test regardless of machine speed. The check covers vectors, matrices, and all adjoint/transpose combinations, for both==andisequal.The existing NaN, signed-zero, stored-zero and dimension-mismatch semantics tests are unchanged apart from running at small sizes.
#769 adds similar timing guards for
findprevandhash; I'll convert those the same way once it merges.🤖 Generated with Claude Code
https://claude.ai/code/session_01LgBHUw9Hp7YW5ub29B4R5y