[GraphBolt][PyG] Modify PyG example with to_pyg_data
#7123
Conversation
|
To trigger regression tests:
|
|
Did you test the performance compare to original pyg example? |
I have updated in the description |
There was a problem hiding this comment.
LGTM overall. What is the performance like compared to the previous version of this example? @RamonZhou
|
What is the blocker for this PR? |
|
There were some changes in master, we should update the branch and resolve any potential issue w.r.t. dtype use of int32. Note: There will be after #7127 is merged. |
|
@RamonZhou It might help improve end-to-end performance if we move the Another thing is to imitate the inference function in our official node_classification example so that we use the dataloader's functionality to load the features. Below, we update the featurestore to be the next layer's features, and no manual feature fetch indexing operation needed. dgl/examples/sampling/graphbolt/node_classification.py Lines 199 to 227 in dbafbe4 |
|
Proposed modification to to_pyg_data. Note that it only works when fanout values are identical across layers. def to_pyg_data(self):
"""Construct a PyG Data from `MiniBatch`. This function only supports
node classification task on a homogeneous graph and the number of
features cannot be more than one.
"""
from torch_geometric.data import Data
if self.sampled_subgraphs is None:
edge_index = None
else:
col_nodes = []
row_nodes = []
prev_num_edges = 0
for subgraph in reversed(self.sampled_subgraphs):
if subgraph is None:
continue
sampled_csc = subgraph.sampled_csc
indptr = sampled_csc.indptr
indices = sampled_csc.indices
expanded_indptr = expand_indptr(
indptr, dtype=indices.dtype, output_size=len(indices)
)
# Drop the edges sampled for dst nodes in later layers like PyG.
my_slice = slice(prev_num_edges, expanded_indptr.size(0))
prev_num_edges = indices.size(0)
col_nodes.append(expanded_indptr[my_slice])
row_nodes.append(indices[my_slice])
col_nodes = torch.cat(col_nodes)
row_nodes = torch.cat(row_nodes)
edge_index = torch.stack((row_nodes, col_nodes)) |
This suggestion can probably improve the inference performance but here we hope not to modify the PyG model too much because if PyG users want to use our GB dataloader, they won't want to do much modification to their original model. |
|
PyG original example uses different fanouts compared to this example: Using |
|
If we are moving features to the GPU, then why not do the same for the graph as well; as usually, the graph takes even less space than the features? @frozenbugs @Rhett-Ying |
Leaving graph on CPU is consistent with PyG canonical example? |
Then we could still leave it on CPU and pin its memory? I also tested and saw no performance difference between pinning features vs moving them to the GPU. @RamonZhou what performance difference do you see between pinning features vs moving them to the GPU? I believe we can simply pin both the graph and the features and the performance should already be better than PyG original example. Compared to PyG, we can advertise that we are using much less GPU memory because we use pinned system memory while being faster. |
Co-authored-by: Muhammed Fatih BALIN <m.f.balin@gmail.com>
Co-authored-by: Muhammed Fatih BALIN <m.f.balin@gmail.com>
Description
Apply
to_pyg_datain the example and implement layer-wise inference.For one train epoch:
For inference:
Acc after 10 epoch:
Checklist
Please feel free to remove inapplicable items for your PR.
Changes