StackOverflowError in scalartype

[lkdvos]

Currently, the fallback definition of scalartype(::Type) does not seem to function as intended:

function scalartype(T::Type)
    @warn _warn_message(scalartype, T) maxlog = 1
    if applicable(eltype, T)
        return scalartype(eltype(T))
    else
        throw(ArgumentError(_error_message(scalartype, T)))
    end
end

In particular, there is a definition in Base: eltype(::Type) = Any, which ensures that eltype is always applicable, and thus scalartype(eltype(T)) leads to infinite recursion when T = Any. This does not usually show up as a problem, however it means that for new types, the cryptic StackOverflowError appears instead of an actually helpful errormessage.

julia> using VectorInterface

julia> struct Test end

julia> scalartype(Test())
┌ Warning: The function `scalartype` is not implemented for (values of) type `Test`;
│ this fallback will disappear in future versions of VectorInterface.jl
└ @ VectorInterface ~/.julia/packages/VectorInterface/TAlcJ/src/fallbacks.jl:20
ERROR: StackOverflowError:

I can think of a couple solutions:

1. We define scalartype(::Type{Any}) = Any, which attempts to still make everything work without figuring out scalartypes.
2. We define scalartype(::Type{Any}) = throw(MethodError(scalartype, Type{Any})), which will then show the backtrace so you can figure out where the scalartype inference went wrong
3. We combine both by implementing option 1 and adding a warning message.

I would want either 1 or 2, which depends on the question of do we really want to support vectors with Any or not. I feel like explicitly throwing an error is definitely a bit restrictive, so I'd rather go for option 1.

[Jutho]

I would pick 2, as having Any will lead to all kind of problems further along (like there not being a zero(Any)).

[lkdvos]

While this is true, that also holds for Number. In principle, zero is not really part of the required interface, and if I wanted to define a type that has scalartype Any (or Number), and just define the interface without calling zero, everything could just work.
I'm always a bit hesitant to hard-code restrictions, as someone might actually need this to work later down the line, and then they can no longer just implement it.

[araujoms]

I found a bug in another package that is caused by this stack overflow. I was about to open an issue here when I found this one. See iitis/QuantumInformation.jl#105

[lkdvos]

This should be fixed as soon as these changes are tagged.

That being said, @araujoms , this fix only means that you will get a MethodError instead of a StackOverflow, I think your linked issue requires to implement this interface for the JuMP variable system, as I think these do not subtype Number (I might be wrong about this, I have no experience with JuMP myself)

[araujoms]

No, I just tested it with the current git, I still get a stack overflow.

JuMP variables in fact do not subtype Number, but they implement a method for eltype, so your fallback should work.

[lkdvos]

Does this resolve the issue?

[araujoms]

Well it gives my an ArgumentError instead of a stack overflow, but I don't see why don't just let it use eltype and make it work.

[lkdvos]

This is intended behaviour, because it will otherwise just fail further down the line, all methods in VectorInterface are by default only defined for scalartypes that subtype Number, so even if we were to just add a fallback, it still will break:

using VectorInterface
import JuMP
VectorInterface.scalartype(t::Type{<:JuMP.VariableRef}) = t
model = JuMP.Model()
JuMP.@variable(model, rho[1:4, 1:4] in JuMP.PSDCone())

julia> scale(rho, one(scalartype(rho))) # expect this to work
ERROR: MethodError: no method matching scale(::LinearAlgebra.Symmetric{JuMP.VariableRef, Matrix{JuMP.VariableRef}}, ::JuMP.AffExpr)

Because again, all of our methods assume that the scalar type subtypes number.
The idea here is that we cannot guarantee that everything will work, so we do not define it, but you are definitely free to define it yourself.
In particular, we tried to keep the number of methods deliberately small, such that in cases like this it should be relatively straightforward to implement VectorInterface.

This being said, if you need any help getting a VectorInterface implementation set up, I can definitely help you out and give some pointers.

[araujoms]

Thanks for the offer, but the error is not in my package, I just needed a function that it provided. Also, that package seems abandoned, so I don't think its devs will be interested.

[lkdvos]

In principle I would say this is a problem at the level of JuMP, which defines the eltypes but does not implement VectorInterface. There could be a very straightforward package extension somewhere for these things to be compatible, so if someone really needs this it can definitely be done.

[araujoms]

I really need the function, but I'll just use the version from OpenQuantumBase instead, which also didn't work but I managed to fix: USCqserver/OpenQuantumBase.jl#109.

I just thought it would be nice to get the version from QuantumInformation working as well, but it's more trouble than it's worth.