CarloLucibello / GraphNeuralNetworks.jl

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

Weights not included in GNNGraph made from SimpleWeightedDiGraph #85

Closed umbriquse closed 6 months ago

umbriquse commented 2 years ago

Hello, just found your sweet package! I ran into a minor issue when wrapping a SimpleWeightedDiGraph with a GNNGraph. The weights from the DiGraph are not included in the wrapped graph.

Below is an MWE of this behavior. I'm not certain if this was by design.

using Graphs, GraphNeuralNetworks, SimpleWeightedGraphs
function randGraph(graphSize::Int)
       graph = rand(graphSize, graphSize)
       foreach(enumerate(eachcol(graph))) do (idx, col)
             graph[idx, :] .= col
             graph[idx, idx] = 0
       end
       return graph
end
a = randGraph(10)
g = SimpleWeightedDiGraph(a)
b = GNNGraph(g)
b.graph

RETURNS

([1, 1, 1, 1, 1, 1, 1, 1, 1, 2  …  9, 10, 10, 10, 10, 10, 10, 10, 10, 10], [2, 3, 4, 5, 6, 7, 8, 9, 10, 1  …  10, 1, 2, 3, 4, 5, 6, 7, 8, 9], nothing)
CarloLucibello commented 2 years ago

SimpleWeightedGraphs.jl is not supported yet. We can add support if think people it is convenient, although I would avoid adding another dependence to this package if possible.

Right now you can create a weighted graph explicitly passing source, target and weights to the constructor:

julia> g = rand_graph(5, 10, bidirected=true)
GNNGraph:
    num_nodes = 5
    num_edges = 10

julia> s, t = edge_index(g)
([1, 1, 2, 2, 4, 4, 5, 4, 5, 5], [4, 5, 4, 5, 5, 1, 1, 2, 2, 4])

julia> w = rand(5);

julia> w = [w; w] # same weight in both directions;

julia> gweighted = GNNGraph(s, t, w)
GNNGraph:
    num_nodes = 5
    num_edges = 10

julia> gweighted.graph
([1, 1, 2, 2, 4, 4, 5, 4, 5, 5], [4, 5, 4, 5, 5, 1, 1, 2, 2, 4], [0.7349537135631546, 0.33820014607400384, 0.08459031049266375, 0.03523075347974425, 0.8861075693177956, 0.7349537135631546, 0.33820014607400384, 0.08459031049266375, 0.03523075347974425, 0.8861075693177956])

Is this convenient enough for you? Also, right now only GCNConv supports edge weight, is that how you were planning to use the weighted graph?

umbriquse commented 2 years ago

Ah, it's not critical for SimpleWeightedGraphs.jl to be supported in my case, just making sure that this was intentional. GCNConv is how I was planning on using edge weights.

CarloLucibello commented 2 years ago

Reopening this as we don't support construction from WeightedGraphs yet

CarloLucibello commented 1 year ago

This could be implemented as a package extension