CarloLucibello / GraphNeuralNetworks.jl

Graph Neural Networks in Julia
https://carlolucibello.github.io/GraphNeuralNetworks.jl/dev/
MIT License
215 stars 46 forks source link

Problem with InlineStrings.jl #98

Closed oysteinsolheim closed 2 years ago

oysteinsolheim commented 2 years ago

How can I work around the issue in InlineStrings https://github.com/JuliaStrings/InlineStrings.jl/issues/21 ? I don't make any sense of the suggested fix.

For example:

julia> using Flux
julia> using GraphNeuralNetworks

julia> g = rand_graph(2,2)
GNNGraph:
    num_nodes = 2
    num_edges = 2

julia> Flux.batch([g,g])
GNNGraph:
    num_nodes = 4
    num_edges = 4
    num_graphs = 2

julia> using InlineStrings

julia> Flux.batch([g,g])
ERROR: MethodError: defalg(::Vector{Union{}}) is ambiguous. Candidates:
  defalg(v::AbstractArray{<:Union{Missing, Number}}) in Base.Sort at sort.jl:658
  defalg(::AbstractArray{<:Union{Missing, String1, String15, String3, String7}}) in InlineStrings at /home/oystein/.julia/packages/InlineStrings/aWvyB/src/InlineStrings.jl:698
Possible fix, define
  defalg(::AbstractArray{<:Missing})
Stacktrace:
 [1] sort!(v::Vector{Union{}})
   @ Base.Sort ./sort.jl:711
 [2] sort(v::Vector{Union{}}; kws::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ Base.Sort ./sort.jl:770
 [3] sort(v::Vector{Union{}})
   @ Base.Sort ./sort.jl:770
 [4] cat_features(x1::NamedTuple{(), Tuple{}}, x2::NamedTuple{(), Tuple{}})
   @ GraphNeuralNetworks.GNNGraphs ~/.julia/packages/GraphNeuralNetworks/KNr8R/src/GNNGraphs/utils.jl:22
 [5] blockdiag(g1::GNNGraph{Tuple{Vector{Int64}, Vector{Int64}, Nothing}}, g2::GNNGraph{Tuple{Vector{Int64}, Vector{Int64}, Nothing}})
   @ GraphNeuralNetworks.GNNGraphs ~/.julia/packages/GraphNeuralNetworks/KNr8R/src/GNNGraphs/transform.jl:177
 [6] batch(gs::Vector{GNNGraph{Tuple{Vector{Int64}, Vector{Int64}, Nothing}}})
   @ GraphNeuralNetworks.GNNGraphs ~/.julia/packages/GraphNeuralNetworks/KNr8R/src/GNNGraphs/transform.jl:256
 [7] top-level scope
   @ REPL[12]:1
 [8] top-level scope
   @ ~/.julia/packages/CUDA/iDsKe/src/initialization.jl:52
CarloLucibello commented 2 years ago

Does defining the methods in https://github.com/JuliaLang/julia/pull/43426/files work for you?

oysteinsolheim commented 2 years ago

Yes! Thank you! Didn't see the actual implementation there and got confused by the DEFAULT_UNSTABLE. By doing

const DEFAULT_UNSTABLE = QuickSort
julia> Base.Sort.defalg(v::AbstractArray{Union{}}) = DEFAULT_UNSTABLE
julia> Base.Sort.defalg(v::AbstractArray{Missing}) = DEFAULT_UNSTABLE

everything seems to work fine. Thank you again!

CarloLucibello commented 2 years ago

Good! Hope that gets merged soon