Add isequal for adjoint/transpose of sparse matrices - #768
Merged
Conversation
#766 added stored-entry-only `isequal` for `SparseMatrixCSC` and for the `Adjoint`/`Transpose` wrappers of sparse vectors, but not for the wrappers of sparse matrices, which still fell back to the generic elementwise `AbstractArray` method. On two 10^5 x 10^5 matrices with one stored entry each, `isequal(A', B')` took ~14s; it now matches `A' == B'` at ~0.2ms. Parameterize `nzeq` on the elementwise predicate, mirror the `==` methods for `Adjoint`/`Transpose` matrices with `isequal`, and loosen the indexed argument of `nzeq` to `AbstractMatrix`. The latter also fixes a pre-existing `MethodError` in `A' == transpose(B)` for complex `A`, where the adjoint of a transpose does not collapse and yields a nested `Adjoint{<:Any,<:Transpose}`. 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 #768 +/- ##
=======================================
Coverage 84.19% 84.19%
=======================================
Files 13 13
Lines 9400 9403 +3
=======================================
+ Hits 7914 7917 +3
Misses 1486 1486 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ViralBShah
added a commit
that referenced
this pull request
Sep 8, 2026
Follow-up to #766 and #768 (and item 6 of the test-time review). The `@elapsed(...) < 0.1` assertions guarding the stored-entries-only `isequal`/`==` paths are wall-clock thresholds, which are the main flake risk on loaded CI runners. The matrix variants also build `10^5 x 10^5` matrices (an 800 KB column-pointer array each) for seven wrapper combinations. They are replaced with a deterministic check. `sparsematrix_ops.jl` already has a `Counting` eltype whose `==` increments a global counter; this PR gives it `isequal` too and asserts that comparing two sparse arrays performs at most `nnz(A) + nnz(B)` element comparisons. The generic `AbstractArray` fallback would perform `length(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 `==` and `isequal`. 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 `findprev` and `hash`; I'll convert those the same way once it merges. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LgBHUw9Hp7YW5ub29B4R5y Co-authored-by: Viral B. Shah <ViralBShah@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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, which added stored-entry-only
isequalforSparseMatrixCSCand for theAdjoint/Transposewrappers of sparse vectors, but not for the wrappers of sparse matrices. Those still fell back to the generic elementwiseAbstractArraymethod even though==has dedicated methods for them.On two
10^5 x 10^5matrices with one stored entry each:isequal(A', B')isequal(A, B')Changes:
nzeqon the elementwise predicate and route the CSC-vs-wrapper==through the same_iseq(eq, A, B)helper introduced in Makeisequalon sparse vectors and matrices walk stored entries only #766.isequalmethods forAdjoint/Transposeof sparse matrices mirroring the existing==ones.nzeqtoAbstractMatrix. This also fixes a pre-existingMethodErrorinA' == transpose(B)for complexA: the adjoint of a transpose does not collapse for complex eltypes, so it yields a nestedAdjoint{<:Any,<:Transpose}that the old one-level signature rejected. (Real eltypes were unaffected sinceadjoint(::Transpose{<:Real})returns the parent.)isequal/==againstMatrixfor NaN, signed zeros, stored zeros, and conjugation.Test.detect_ambiguities(SparseArrays)remains empty.🤖 Generated with Claude Code
https://claude.ai/code/session_01LgBHUw9Hp7YW5ub29B4R5y