Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New method ambiguity in 1.10: vcat on KeyedArrays #52386

Closed
aplavin opened this issue Dec 4, 2023 · 4 comments
Closed

New method ambiguity in 1.10: vcat on KeyedArrays #52386

aplavin opened this issue Dec 4, 2023 · 4 comments

Comments

@aplavin
Copy link
Contributor

aplavin commented Dec 4, 2023

When running some package tests on the upcoming (1.10rc) Julia version, I noticed a new error due to method ambiguity:

julia> using AxisKeys

julia> vcat(KeyedArray([1,2], x=[:a,:b]), KeyedArray([3], x=[:c]))
# 1.9 result:
1-dimensional KeyedArray(NamedDimsArray(...)) with keys:
   x  3-element Vector{Symbol}
And data, 3-element Vector{Int64}:
 (:a)  1
 (:b)  2
 (:c)  3

# 1.10 result:
ERROR: MethodError: vcat(::KeyedArray{Int64, 1, NamedDimsArray{(:x,), Int64, 1, Vector{Int64}}, Base.RefValue{Vector{Symbol}}}, ::KeyedArray{Int64, 1, NamedDimsArray{(:x,), Int64, 1, Vector{Int64}}, Base.RefValue{Vector{Symbol}}}) is ambiguous.

Candidates:
  vcat(A::Union{KeyedArray{T, 1, AT, RT}, KeyedArray{T, 2, AT, RT}} where {T, AT, RT}, B::Union{KeyedArray{T, 1, AT, RT}, KeyedArray{T, 2, AT, RT}} where {T, AT, RT}, Cs::AbstractVecOrMat...)
    @ AxisKeys ~/.julia/packages/AxisKeys/QsNqy/src/functions.jl:147
  vcat(X1::Union{Number, AbstractVecOrMat{<:Number}}, X::Union{Number, AbstractVecOrMat{<:Number}}...)
    @ SparseArrays ~/.julia/juliaup/julia-1.10.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/SparseArrays/src/sparsevector.jl:1235
  vcat(A::Union{KeyedArray{T, 1, AT, RT}, KeyedArray{T, 2, AT, RT}} where {T, AT, RT}, B::AbstractVecOrMat, Cs::AbstractVecOrMat...)
    @ AxisKeys ~/.julia/packages/AxisKeys/QsNqy/src/functions.jl:147
  vcat(A::AbstractVecOrMat, B::Union{KeyedArray{T, 1, AT, RT}, KeyedArray{T, 2, AT, RT}} where {T, AT, RT}, Cs::AbstractVecOrMat...)
    @ AxisKeys ~/.julia/packages/AxisKeys/QsNqy/src/functions.jl:147
  vcat(V::AbstractVector...)
    @ Base abstractarray.jl:1620
  vcat(A::AbstractVecOrMat{T}...) where T
    @ Base abstractarray.jl:1691
  vcat(V::AbstractVector{T}...) where T
    @ Base abstractarray.jl:1621

Possible fix, define
  vcat(::KeyedArray{T, 1}, ::KeyedArray{T, 1}, ::Vararg{AbstractVector{T}}) where T<:Number

The vcat method list is quite long so it's hard to compare and tell – whether any new methods actually appeared in 1.10, or the ambiguity resolution algorithm changed.

What I find weird in the message is that some of the candidates are clearly less specific than others. This makes me think that maybe there's some bug in the resolution?

If not and everything goes as expected, then what the solution should be? Are array types really expected to define vcat(::MyArray{T}, ::MyArray{T}) where T<:Number whenever they define vcat(::MyArray{T}, ::MyArray{T})? Seems strange to single out Number here.

@jishnub
Copy link
Contributor

jishnub commented Dec 4, 2023

Possibly duplicate of #52263. The offending method is added by SparseArrays through type-piracy

@KristofferC
Copy link
Member

Does adding the recommended method solve it?

@vtjnash
Copy link
Member

vtjnash commented Dec 4, 2023

This should be re-opened on the SparseArrays repo, since the offending pirating method is added there.

@aplavin
Copy link
Contributor Author

aplavin commented May 14, 2024

In case others find this issue and want a simple reliable workaround – here it is:

Base.delete_method.(filter(
    m -> nameof(m.module) == :SparseArrays && m.sig == Tuple{typeof(vcat), Union{Number,AbstractVecOrMat{<:Number}}, Vararg{Union{Number,AbstractVecOrMat{<:Number}}}},
    methods(vcat)
))
Base.delete_method.(filter(
    m -> nameof(m.module) == :SparseArrays && m.sig == Tuple{typeof(hcat), Union{Number,AbstractVecOrMat{<:Number}}, Vararg{Union{Number,AbstractVecOrMat{<:Number}}}},
    methods(hcat)
))

Put this code into your script/code anywhere after package imports. It deletes those pirating methods defined in SparseArrays, getting rid of such ambiguities. At the same time, it doesn't interfere with vcat/hcat on actual sparse arrays – they continue returning sparse arrays.

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

No branches or pull requests

4 participants