Comments (11)
For RGCN, I think the weight W
should not be in the message signature, and message function should directly reference it (for example, using closure).
Here is what the code might look like:
class MessageFunc(nn.Module):
def __init__(self):
self.w = torch.Tensor(num_relations, in_feat, out_feat)
def forward(self, src, edge):
return torch.bmm(src['h'], self.w[edge['type']])
Then the propagation looks like:
g.update_all(msg_func, fn.sum(), update_func)
And we need to lower the spmv executor from update_all
, send_and_recv
api to recv
api so that all builtin reduce can be done as a spmv from edge space to node space. And if message func is also builtin, we can provide a shortcut to rewrite as a spmv from node space to node space. This is related to executor redesign @jermainewang and I previously discussed.
from dgl.
@BarclayII @ylfdq1118 Thanks for the proposal. I agree that it's better to use incident lists to support multi-graphs.
Each edge has a unique ID. It shouldn't be a big problem for the API of DGLGraph
, such as get_e_repr
and set_e_repr
.
I wonder if it's too expensive to return lists of lists for get_eids
, batch_in_edges
and batch_out_edges
. What is the problem of returning them as a single list?
How do you want to use the filtering function with other APIs? One possible way is to pass them to the APIs, such as update_all
, to remove some nodes and edges on the fly during the computation. The question is whether this is necessary given the subgraph API? One advantage may be that filtering at the runtime may give more information to users to decide when to sample an edge or a vertex.
from dgl.
@BarclayII @ylfdq1118 Thanks for the proposal. I agree that it's better to use incident lists to support multi-graphs.
Each edge has a unique ID. It shouldn't be a big problem for the API of
DGLGraph
, such asget_e_repr
andset_e_repr
.
The current semantics of get_e_repr and set_e_repr takes in pairs of source/target vertices. Under this semantics, if the number of edges between those pairs are different, then the number of rows of the edge tensor will be different than the number of source/target pairs. Users will have to align the rows to which pair of nodes in this case, and we believe that doing so would be quite a hassle. They may as well use get/set_e_repr_by_id in this case. So we believe that adapting the current get/set_e_repr to multigraph is subtle and/or redundant.
I wonder if it's too expensive to return lists of lists for
get_eids
,batch_in_edges
andbatch_out_edges
. What is the problem of returning them as a single list?
My mistake. I checked the implementation of batched inbound/outbound edge queries and there should be no problem, since we are returning a list of (source, target, edge ID) tuples. I removed that proposal.
That being said, do we need to support batched queries of predecessors/successors? I feel that this should be a nice and convenient addition, but I'm unsure whether we need it right now or in the near future.
How do you want to use the filtering function with other APIs? One possible way is to pass them to the APIs, such as
update_all
, to remove some nodes and edges on the fly during the computation. The question is whether this is necessary given the subgraph API? One advantage may be that filtering at the runtime may give more information to users to decide when to sample an edge or a vertex.
The point of having the filtering functions is to support selecting a subset of nodes/edges using predicates. For example, one might want to "sample all the nodes with the maximum hidden state component greater than 1" (as in my second example). The resulting node subset can as well change dynamically.
That being said, I believe that users can do the same thing manually (again in my second example). But personally I would prefer our framework to somehow support querying by predicates conveniently, by returning the selected node/edge IDs, subgraphing, or any other way.
from dgl.
Could some one briefly describes what is an "incident list"? What is the difference with the current adjacency list here?
from dgl.
My understanding is that "incident list" is more or less the same as "adjacency list". The only difference is that it allows duplicated edges. I simply use their term. @BarclayII @ylfdq1118 is my understanding correct?
from dgl.
Technically "adjacency list" is about "which nodes are the direct successors to the given node", while "incidence list" is about "which edges are outbound from the given node". So the C++ storage itself also contains everything we need for incidence lists. This is good news since we don't need to change the storage (sorry I missed that), and we only need to tweak our code to allow duplicate succ
's.
from dgl.
If we don't have any further problems then I assume that I'll go ahead and implement these changes?
from dgl.
In previous discussion with @jermainewang , we think the query API is running at relatively high level (meaning in python code) because query API involves frame storage. But since GraphIdx is decoupled from storage, I am not sure about the exact structure we are going to use in GraphIdx for efficient edge type querying (maybe just one extra layer of indirection). Also, we have concerns about whether MultiGraphIdx should be a separate C++ class.
I think we need to be crystal clear about the design since GraphIdx affects a lot of stuff. So just hold on for a while until I get time to work on this. Also, we need to discuss more and push a detailed design doc on GraphIdx for others to comment on.
from dgl.
Re query API: I'm thinking of an extra layer of indirection too. Basically right now the frame storage only allows indexing by column. We can additionally have a "row indexer" for row slicing, following the same design as pandas.DataFrame
. Then we only need to pass in the row-sliced "subframe" into the predicate.
Re MultiGraphIdx: I'm not sure what's the concern of having GraphIdx handling both simple- and multi-graph. In my mind GraphIdx can be mostly kept as it is right now (except edge_id
and edge_ids
). But I do agree that we need to inspect it in more detail.
from dgl.
I see. I think the multi-edge design should be at the same level as C++ GraphIndex. That's why I'm really surprised when I heard from Lingfan that your guy's design has something with the frame storage. Currently, Lingfan and I are too busy (will finally be released after tomorrow). But I think there are several things worth thinking:
- Read through the current GraphIndex class and play with it. You need to understand how it is used in the python side. The python binding is already complete so you can actually run it in python.
- In the high level, start from the application/example, think about how the python side API will look like for users.
- From the high level APIs, summarize a narrow low-level interface that needs to be implemented in C++.
- Given the low-level APIs, how does the other existing packages implement them? (e.g. BGL, igraph, SGraph, graph-tool, etc.)
From then, we can try to formalize the design and implement it.
from dgl.
A cleaned up proposal is available at
https://docs.google.com/document/d/12gSTTMUNGVt-ZJkj4VCxUUOYVdSh07tUHKcM3_NU7-c/edit?usp=sharing
Closing this issue for now.
from dgl.
Related Issues (20)
- Deepwalk Network Adaptability HOT 1
- [GraphBolt] high memory consumption when building GPU version HOT 11
- Migrating CMake build from `FindCUDA` to `FindCUDAToolkit` HOT 3
- [GraphBolt] Tensor Constructor for TorchBasedFeatureStore HOT 2
- DGL dataloader returns batch size None HOT 3
- [GraphBolt] extend regression tests for pipelining optimization HOT 3
- [GraphBolt] Dataset dtypes HOT 12
- [DistGB] `IdMap` support `int64` ids only HOT 2
- How should I install a compatible DGL cuda enabled version in Colab for CUDA 12.2? HOT 3
- Unable to install version 2.0 from Google Colab HOT 5
- [GraphBolt] Compress type_per_edge array so that instead of COO, we use CSC. HOT 2
- [DistGB] remove `utils.toindex()` from DistDGL
- [DistGB] convert DGL partitions to GraphBolt format directly after partitioning
- Unable to add `dgl` as Python dependency in TOML specifications HOT 2
- Models in "5.2 Edge Classification/Regression" do not use edge features for edge classification tasks, or the tutorials is unclear about it HOT 2
- GraphBolt HOT 6
- [DistGB] upgrade torch to 2.0.0 in CI
- Question about the incompatibility of cuda12.1 version with cuda12.0 version on the server HOT 1
- [DistGB] re-implement IdMap with torch instead of numpy
- [DGL][CUDA] `examples/sampling/node_classification.py --mode=gpu` is slow.
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from dgl.