Method ambiguity error shows irrelevant methods

[aplavin]

This example was originally reported in #52386: some methods shown in the MethodError are clearly less specific than others and are irrelevant for the error itself.

Can/should it only show methods that actually constitute the ambiguity?

julia> using AxisKeys

julia> vcat(KeyedArray([1,2], x=[:a,:b]), KeyedArray([3], x=[:c]))
# 1.10-rc result (current release 1.9 works without errors):
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

[vtjnash]

I think this code calls methods and then does some similarity scoring. But calling methods_including_ambiguous first would probably be a better list to use for this (if that list is non-empty)

[lgoettgens]

I think this code calls methods and then does some similarity scoring. But calling methods_including_ambiguous first would probably be a better list to use for this (if that list is non-empty)

This is already called and the result is passed to showerror_ambiguous, see

julia/base/errorshow.jl

Lines 252 to 255 in 456951f

	meth = methods_including_ambiguous(f, arg_types)
	if isa(meth, MethodList) && length(meth) > 1
	return showerror_ambiguous(io, meth, f, arg_types)
	end

Furthermore, there is a test that checks the existence of these "irrelevant" methods in the error message, see the third line in

julia/test/ambiguous.jl

Lines 43 to 46 in 456951f

	@test occursin(" ambig(x, y::Integer)\n @ $curmod_str", errstr)
	@test occursin(" ambig(x::Integer, y)\n @ $curmod_str", errstr)
	@test occursin(" ambig(x::Number, y)\n @ $curmod_str", errstr)
	@test occursin("Possible fix, define\n ambig(::Integer, ::Integer)", errstr)

[vtjnash]

That one is not unrelated though, since even if you deleted the second one, the first and third would still be ambiguous

[lgoettgens]

Just as a note: The current printing has been introduced in #32776.

So you would define an "irrelevant method" as one that does not constitute any ambiguities with all of the other methods?

[vtjnash]

That is what OP asked for as relevant:

Can/should it only show methods that actually constitute the ambiguity?

[lgoettgens]

As methods_including_ambiguous returns these irrelevant methods, I would consider this a bug in methods_including_ambiguous, right?

[vtjnash]

Well, they are indeed ambiguous, so it is a bit ambiguous what it would mean to call them irrelevant to include when explicitly asked to return a list of ambiguities

[vtjnash]

It may be worth noting that "less specific than the others" implies the OP thinks of this as a total order predicate. It is not a total ordering predicate, so such a statement is not as clearly defined in such a situation.

[aplavin]

It may be worth noting that "less specific than the others" implies the OP thinks of this as a total order predicate.

I'm the OP and definitely did not imply that. Sorry if that's the impression!

Of course the order is partial, and for given argument types there's a "pareto front" of the most specific methods – those that aren't "dominated" by even more specific ones. If the front consists of one method, there's no ambiguity, if multiple methods – then this error.
In the original example, let's look at the last three:

 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

Surely, the last one is more specific than the other two. So only the last of these three can participate in any ambiguity here, the others are irrelevant.

[vtjnash]

there's a "pareto front"

Again, this is a notion that is from your assumption of total order, and does not hold here. Such a "pareto front" is not present in a partial order, such as is being printed here. Your example indeed shows this, as the methods are sorted (in the absence of ambiguities) from most specific to least--and yet as you pointed out, the method you thought should the most specific is the last one among the ambiguities detected. You can almost recover a total ordering from subtyping, but not entirely and there are counter-examples in the tests.

[aplavin]

Again, this is a notion that is from your assumption of total order, and does not hold here. Such a "pareto front" is not present in a partial order, such as is being printed here.

Maybe we are meaning different things by the same names?
"Total order" is where any two elements can be compared, and there it doesn't make sense to talk about pareto frontiers – there is just a minimum in every set. It's for "partial orders" where some elements cannot be compared (as in method specificity) the "pareto front" concept makes sense.

[vtjnash]

Sorry, yes, I meant to write it is not a partial order either. Just a general cyclic graph.

[aplavin]

Are there really any cycles in methods by specificity? Like, A is more specific than B, B than C, and C than A.

Anyway, in the provided example at least these two methods are completely irrelevant to the ambiguity and shouldn't really be shown if possible:

 vcat(V::AbstractVector...)
    @ Base abstractarray.jl:1620
  vcat(A::AbstractVecOrMat{T}...) where T
    @ Base abstractarray.jl:1691

[vtjnash]

Yes, your provided example itself appears to such a cycle. You can tell because the possible fix would have had to be different if those were not included--thus not irrelevant. You may want to try building the n*n matrix of morespecific for those entries above to see for yourself

[aplavin]

Could you please be more concrete? I don't see any cycle at all in the provided example. And I still don't understand how the proposed fix is affected by

 vcat(V::AbstractVector...)
  vcat(A::AbstractVecOrMat{T}...) where T

methods.

[aplavin]

closed this as not planned

Well, it's unfortunate that improving error messages doesn't appear to be a priority here. I mean, I can understand this viewpoint myself, but for ease of use, playing with the language, and debugging problems, the cleaner error messages are the better – best if they contain all relevant information and no irrelevant details.

[aplavin]

Another example where the error message shows irrelevant methods:

  MethodError: similar(::StructArray{@NamedTuple{a::Float64, b::Bool}, 2, @NamedTuple{a::Matrix{Float64}, b::Matrix{Bool}}, Int64}, ::Type{ComplexF64}, ::Tuple{Base.OneTo{Int64}}) is ambiguous.
  
  Candidates:
    similar(s::StructArray, S::Type, sz::Tuple{Union{Integer, AbstractUnitRange}, Vararg{Union{Integer, AbstractUnitRange}}})
      @ StructArrays ~/.julia/dev/StructArrays/src/structarray.jl:299
    similar(s::StructArray, S::Type, sz::Tuple{Vararg{Union{Integer, Base.OneTo}}})
      @ StructArrays ~/.julia/dev/StructArrays/src/structarray.jl:299
    similar(a::AbstractArray, ::Type{T}, dims::Tuple{Union{Integer, Base.OneTo}, Vararg{Union{Integer, Base.OneTo}}}) where T
      @ Base abstractarray.jl:839
    similar(A::AbstractArray, ::Type{T}, shape::Tuple{Vararg{Union{Integer, Base.OneTo, SOneTo}}}) where T
      @ StaticArrays ~/.julia/packages/StaticArrays/EHHaF/src/abstractarray.jl:145
  
  Possible fix, define
    similar(::StructArray, ::Type, ::Tuple{Union{Integer, Base.OneTo}, Vararg{Union{Integer, Base.OneTo}}})

Here, method #2 is clearly more specific than #4, so #4 shouldn't really be shown.

[vtjnash]

It looks like #1 is ambiguous with 2,3,4, so they are all in the same SCC cycle. There is no transitive order to morespecific, so having 2 be morespecific than 4 is not relevant to computing the list of ambiguities