Fix method ambiguities in SparseArrays #30120
Conversation
| @inline capturescalars(f, mixedargs::Tuple{SparseVecOrMat, Ref{Type{T}}, Vararg{Any}}) where {T} = | ||
| capturescalars((a1, args...)->f(a1, T, args...), (mixedargs[1], Base.tail(Base.tail(mixedargs))...)) | ||
| @inline capturescalars(f, mixedargs::Tuple{Union{Ref,AbstractArray{0}}, Ref{Type{T}}, Vararg{Any}}) where {T} = | ||
| @inline capturescalars(f, mixedargs::Tuple{Union{Ref,AbstractArray{<:Any,0}}, Ref{Type{T}}, Vararg{Any}}) where {T} = |
There was a problem hiding this comment.
This was irrelevant to this PR but I suppose AbstractArray{0} was a typo?
| @test ((_, x) -> x).(Int, spzeros(3)) == spzeros(3) | ||
| @test ((_, _, x) -> x).(Int, Int, spzeros(3)) == spzeros(3) | ||
| @test ((_, _, _, x) -> x).(Int, Int, Int, spzeros(3)) == spzeros(3) | ||
| @test_broken ((_, _, _, _, x) -> x).(Int, Int, Int, Int, spzeros(3)) == spzeros(3) |
There was a problem hiding this comment.
It looks like capturescalars has type inference issue when it has to recurse a lot (> 3). I wounder if capturescalars could be implemented more naturally (for humans and maybe for Julia as well?) if you used @generated function. Any reason why capturescalars is implemented based on recursion?
| @@ -1006,10 +1007,6 @@ end | |||
| _copyto!(parevalf, dest, passedsrcargstup...) | |||
| end | |||
|
|
|||
| struct CapturedScalars{F, Args, Order} | |||
There was a problem hiding this comment.
git grep CapturedScalars shows me that there is no code using it. Maybe some remnant from experimenting with capturescalars? This was irrelevant to this PR but I thought it's OK to remove it here (as I'm touching capturescalars).
| @@ -973,6 +973,7 @@ function _copy(f, args...) | |||
| parevalf, passedargstup = capturescalars(f, args) | |||
| return _copy(parevalf, passedargstup...) | |||
| end | |||
| _copy(f) = throw(MethodError(_copy, (f))) # avoid method ambiguity | |||
There was a problem hiding this comment.
(f) === f, should be (f,)
There was a problem hiding this comment.
Thanks for catching this. I fixed it and added a test.
* Remove unused struct CapturedScalars * Fix method ambiguities in SparseArrays * Fix HigherOrderFns._copy(f) (cherry picked from commit f10530e)
* Remove unused struct CapturedScalars * Fix method ambiguities in SparseArrays * Fix HigherOrderFns._copy(f) (cherry picked from commit f10530e)
* Remove unused struct CapturedScalars * Fix method ambiguities in SparseArrays * Fix HigherOrderFns._copy(f) (cherry picked from commit f10530e)
StefanKarpinski
added
status:triage
JeffBezanson
removed
backport 1.0
status:triage
I added a few methods to resolve method ambiguity in SparseArrays.jl. I also included the test using
detect_ambiguitiesas I did for LinearAlgebra.jl in #28749 and #28816.