Skip to content

Fix all method ambiguities; enable Aqua's ambiguity check - #168

Merged
ViralBShah merged 4 commits into
v11from
vs/fix-ambiguities
Oct 27, 2026
Merged

ViralBShah merged 4 commits into
v11from
vs/fix-ambiguities

Conversation

@ViralBShah

@ViralBShah ViralBShah commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Test.detect_ambiguities(SuiteSparseGraphBLAS; recursive = true) goes from 212 to 0, and the Aqua CI job now runs Aqua.test_all(SuiteSparseGraphBLAS) with no exceptions.

Main causes:

  • 3-arg * (≈100 pairs vs LinearAlgebra's A * B * C): the semiring argument was untyped. It is now op::SemiringSpec = (+, *), where SemiringSpec = Union{Tuple, TypedSemiring}.
  • Untyped scalar *(x, A) / *(A, x) (≈55): removed; the x::Number methods next to them remain. These also caught non-scalars, so A * sparsevector and A * view(x, …) were ambiguity errors and would otherwise have run as an elementwise scalar apply. They now work.
  • inferbinarytype / indexunaryop (52): the @eval'd (::Any, ::Any, ::typeof(op)) methods are now (::Type, ::Type, ...). The array forms already forward to the type forms.
  • Vector getindex (≈20): the catch-all getindex(u::AbstractGBVector, I) is replaced with typed methods for AbstractVector / AbstractUnitRange (plus Bool variants), all still calling extract. Scalar getindex now covers every AbstractGBVector, so pack(v)[i] works (it was a MethodError).
  • Iterator indexing: A[I::GxBIterator] is restricted to GraphBLAS and dense matrices; it was defined on every AbstractArray.

Smaller fixes:

  • eunion/eunion! shortcut forms take α/β::Number. The explicit 5-arg form is unchanged.
  • The per-type _assign takes x::$T, like _subassign.
  • Shallow resize! takes Integer arguments.
  • Tie-breakers for convert (into shallow vectors), _promotefill (Missing vs Nothing → missing), apply! (two arrays, transposed first: throws), hcat (now needs ≥1 argument), typedmonoid, build!, reshape and u' * x (real dense x, which returns 1×1 like u' * v).

Behavior check against v11: scalar *, A*x, A*A, *(A, A, (+, *)), D*A, v[1:2]/v[[2,1]]/v[:], hcat, reshape and eunion all give the same results. Four cases that errored before now work: v' * x, A * sparse(x), A * view(...), and pack(v)[2].

Aqua (all checks, including ambiguities) and the full Pkg.test() pass locally on nightly. Small tests for the four newly working cases are added to the existing mul.jl and gbarray.jl testsets.

🤖 Generated with Claude Code

ViralBShah and others added 2 commits October 27, 2026 18:34
Test.detect_ambiguities went from 212 to 0:
- Type the semiring argument of 3-arg * so it can't clash with A * B * C.
- Drop the untyped scalar * methods; the Number ones remain. This also stops
  A * sparsevector / A * view(...) from being treated as a scalar apply.
- Type the @eval'd inferbinarytype/indexunaryop methods on ::Type.
- Typed vector getindex methods instead of a catch-all; scalar getindex now
  covers all AbstractGBVectors (shallow vectors included).
- Restrict iterator indexing to GraphBLAS and dense matrices.
- Narrow eunion's convenience forms, _assign's scalar, shallow resize!.
- Tie-breakers for convert, _promotefill, apply!, hcat, typedmonoid, build!,
  reshape and u' * x.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.78788% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.38%. Comparing base (1fb74bb) to head (460f319).

Files with missing lines Patch % Lines
src/operators/selectops.jl 40.00% 3 Missing ⚠️
src/iterator.jl 50.00% 2 Missing ⚠️
src/abstractgbarray.jl 88.88% 1 Missing ⚠️
src/solvers/cholmod.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              v11     #168      +/-   ##
==========================================
+ Coverage   67.38%   68.38%   +0.99%     
==========================================
  Files          54       54              
  Lines        4485     4494       +9     
==========================================
+ Hits         3022     3073      +51     
+ Misses       1463     1421      -42     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ViralBShah and others added 2 commits October 27, 2026 19:09
- Dense matrix (or its transpose) times a GraphBLAS vector also matched
  SparseArrays' triangular/adjoint * sparse vector methods: add tie-breakers.
- GB_CHOLMOD's adjoint-FactorComponent \ took VecOrMat too, duplicating
  SparseArrays' own method; restrict it to AbstractGBArray.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every method added or narrowed for the ambiguity fixes is now exercised.
Boolean-vector indexing selects the true positions (it threw before), and
the integer-tuple reshape tie converts dims to Int.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ViralBShah
ViralBShah merged commit aa763ff into v11 Oct 27, 2026
8 checks passed
@ViralBShah
ViralBShah deleted the vs/fix-ambiguities branch October 27, 2026 19:41
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.

1 participant