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

Weights not included in GNNGraph made from SimpleWeightedDiGraph #85

Closed
umbriquse opened this issue Dec 22, 2021 · 4 comments · Fixed by #371
Closed

Weights not included in GNNGraph made from SimpleWeightedDiGraph #85

umbriquse opened this issue Dec 22, 2021 · 4 comments · Fixed by #371
Labels
good first issue Good for newcomers

Comments

@umbriquse
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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

@CarloLucibello
Copy link
Member

This could be implemented as a package extension

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants