FluxML / GeometricFlux.jl

Geometric Deep Learning for Flux
https://fluxml.ai/GeometricFlux.jl/stable/
MIT License
348 stars 30 forks source link

Node2vec prototype #247

Closed jarbus closed 2 years ago

jarbus commented 2 years ago

Still very much a work in progress, right now the example doesn't run because GraphSignals, Graphs GeometricFlux, have name collisions, and I'm also getting the error LoadError: UndefVarError: neighbors not defined even though we import Graphs in src/GeometricFlux.jl. I'm new to Julia package development so not entirely sure what's going on here. However, I tested the code by including all functionality in a script and all the functions work as intended.

jarbus commented 2 years ago

@yuehhua

yuehhua commented 2 years ago

As for the neighbors, you can use Graphs.neighbors instead.

jarbus commented 2 years ago

As for the neighbors, you can use Graphs.neighbors instead.

That would make sense, but even when perform such a replacement, I get

WARNING: both GeometricFlux and Graphs export "neighbors"; uses of it in module Main must be qualified

ERROR: UndefVarError: neighbors not defined

Any ideas @yuehhua?

yuehhua commented 2 years ago

You should call Graphs.neighbors instead of neighbors.

jarbus commented 2 years ago

You should call Graphs.neighbors instead of neighbors.

I am! That's the thing. It's still having namespace issues even when I call Graphs.neighbors

https://github.com/jarbus/GeometricFlux.jl/blob/node2vec/src/graph_embedding/node2vec.jl#L108-L124

jarbus commented 2 years ago

@yuehhua

yuehhua commented 2 years ago

Sorry, I'm busy these days. I checked calling Graphs.neighbors and have GeometricFlux imported in my side. It works well. Maybe I can check what's the difference between us.

jarbus commented 2 years ago

Oh I figured it out! I was using the master branch instead of the node2vec branch somehow when adding the package.

jarbus commented 2 years ago

@yuehhua I'm thinking about switching to using GraphSignals.FeaturedGraph, in order to get this to work seamlessly on Weighted Graphs and to add some consistency with the rest of the package.

Some notes:

yuehhua commented 2 years ago

@jarbus

GeometricFlux is currently incompatible with the newest version of GraphSignals

Yes, I am currently trying to make them compatible.

Not a priority, but it would be nice to add has_edge, inneighbors outneighbors and neighbors to GraphSignals.

So, you want FeaturedGraph to support has_edge, inneighbors, outneighbors and neighbors?

jarbus commented 2 years ago

@yuehhua It's no longer needed, as I figured out a workaround using the FeaturedGraph's adjacency matrix, but for anything regarding graph traversal, functions like outneighbors and the like are really handy and make the code more readable.

yuehhua commented 2 years ago

I have rebased this branch to the new release. Please update to your local repo.

jarbus commented 2 years ago

@yuehhua I'm trying to switch to a FeaturedGraph backend for node2vec, and think that it would be really helpful to include functions like edges and neighbors as part of the GraphSignals API, as it's quite inelegant to iterate over them using the adjacency matrix.

yuehhua commented 2 years ago

Does yuehhua/GraphSignals.jl#76 fit your need?

jarbus commented 2 years ago

Does yuehhua/GraphSignals.jl#76 fit your need?

@yuehhua Unfortunately not. node2vec uses a sampling method called 'AliasSampling, which precomputes transition probabilities to allow for constant-time sampling, and also uses two parameters,pandq`, to control how far the random walk strays from the starting node, so I can't use just any sampling/random walk method. I've reimplemented neighbor and edge functionality but it would be nice to have it built-in to GraphSignals

yuehhua commented 2 years ago

OK, I will put edges and neighbors as part of the GraphSignals API in yuehhua/GraphSignals.jl#77.

jarbus commented 2 years ago

@yuehhua Thanks for all of your help so far :) I have another question, which will hopefully be my last: The node2vec implementation to support both directed and weighted graphs, directed seems simple enough now that I can use neighbors(g, v;dir=:out), but is there an interface for edge weights?

jarbus commented 2 years ago

Actually, I just pushed using the adjacency matrix of the SparseGraph, which looks like graph(fg).S[v,:] |> findnz to return neighbor ids and their weights. I'm able to run random walks on the cora dataset pretty quickly, and I have tests confirming that it works on the karate club network, so I think node2vec is almost ready for merging!

yuehhua commented 2 years ago

Please use yuehhua/GraphSignals.jl#78

jarbus commented 2 years ago

We should be good to go. I benchmarked on Cora, takes about 0.7 seconds to conduct 27k walks, of length 80, seems about 15 times faster than the python implementation from the original paper: https://github.com/aditya-grover/node2vec

yuehhua commented 2 years ago

@jarbus Thank you for contributions. Closes #232