First PR for WholeGraph 23.06 refactor and add RAPIDS CI scripts #16

dongxuy04 · 2023-05-10T03:26:40Z

Refactor for WholeGraph, some updates:

More WholeMemory types.
WholeMemory Embedding.
WholeMemory Embedding Cache support.
WholeMemory Embedding Sparse Optimizer.
DLPack export for WholeMemory.
Torch C10 API removed.
Packaging for conda.
RAPIDS CI scripts.

BradReesWork · 2023-05-10T13:49:13Z

@dongxuy04 you need to add a conda environment yml file under "conda/environments"

dongxuy04 · 2023-05-10T14:03:09Z

@dongxuy04 you need to add a conda environment yml file under "conda/environments"

@BradReesWork added conda environment yml files generated by rapids-dependency-file-generator.

seunghwak · 2023-05-10T16:56:26Z

Does WholeGraph provide C-API? or C++-API? All the public headers under the cpp/include directory has either .h or .cuh extensions and should I assume this is to support C users?

dongxuy04 · 2023-05-10T23:31:54Z

Does WholeGraph provide C-API? or C++-API? All the public headers under the cpp/include directory has either .h or .cuh extensions and should I assume this is to support C users?

Yes, for host API part, .h files provide C-API instead of C++-API, the initial consideration is not to introduce ABI issues. For device headers, .cuh aims to provide developing interface on WholeMemory, so it expose C++ device API to access WholeMemory.

BradReesWork · 2023-05-11T16:57:00Z

@dongxuy04 can you rename build_components.sh to just build.sh to match other repos?

dongxuy04 · 2023-05-12T04:10:55Z

@dongxuy04 can you rename build_components.sh to just build.sh to match other repos?

@BradReesWork I have renamed build_components.sh to build.sh, and updated scripts called this script.

MatthiasKohl · 2023-05-12T13:54:12Z

@BradReesWork this PR has ~50K touched lines, is there something in particular you want me to review?

stadlmax · 2023-05-12T14:11:35Z

pylibwholegraph/pylibwholegraph/torch/graph_ops.py

For another time: what is our strategy regarding GNN ops? @teju85 @BradReesWork
cugraph-ops always has been told to stay closed source. This is mostly duplicating the work we have in cugraph-ops. Further functionalities are duplicating stuff we have in cugraph-pyg and cugraph-dgl.
As discussed already two times offline, I thought the whole point of this refactoring is to make WholeGraph a storage-only solution.

MatthiasKohl · 2023-05-12T14:31:18Z

For every file and kernel under cpp/src/graph_ops, can we get a description of:

what is the goal? what model this operation would be used for?
was cugraph-ops considered? if yes, why was it not possible to use cugraph-ops/what is missing from cugraph-ops?

MatthiasKohl · 2023-05-12T14:32:40Z

tagging @BradReesWork since you asked for scope of duplication of operations and @dongxuy04 for visibility for the above comment

…into branch-23.06

teju85 · 2023-05-12T14:46:35Z

I understand that this PR only focuses on refactor of WG into a more RAPIDS-like workflow. But I too still want to mention the cugraph-ops integration into WG and urge to put this on our immediate list of things todo for WG.

I agree with both Max's/Matthias' requests. cugraphops should already provide all (if not most) of those operations. So, it's not worth the duplication. I have filed issue #17 for this.

dongxuy04 · 2023-05-12T14:58:44Z

For every file and kernel under cpp/src/graph_ops, can we get a description of:

what is the goal? what model this operation would be used for?

was cugraph-ops considered? if yes, why was it not possible to use cugraph-ops/what is missing from cugraph-ops?

These files are legacy implementations left over from previous versions of WholeGraph code. Agreed that we need to remove these duplication and actually this is in processing. The goal might be to get most of these files replaced by cuGraph ops.

Here are the goal of the files under cpp/src/graph_ops.

append_unique*: AppendUnique op, it is an optimizing OP for ids between sampling of two layers. The action is append sampled neighbor nodes to center nodes, and the do unique while keep center nodes unchanged. May need to keep as it is not related to cuGraphOps.
csr_add_self_loop*: add self loop to sampled csr sub graph. Not sure if there is this kind of ops. If yes, we may remove that.
edge_weight_softmax*, gspmm_csr_weighted*, spadd_gat_csr*: ops related to GAT model, currently GAT module from cuGraphOps is integrating, will be removed after confirmed.
spmm_csr_no_weight*: Ops related to GraphSAGE op and GCN op, currently GraphSAGE module from cuGraphOps is integrated, will be removed later.

dongxuy04 · 2023-05-19T06:36:09Z

@MatthiasKohl @stadlmax @teju85 @BradReesWork Agreed that it doesn't worth duplication. I have verified that similar ops in cuGraphOps works and removed the duplicated ops under cpp/src/graph_ops.

BradReesWork · 2023-05-23T16:08:48Z

the "cmake" folder should be under the cpp directory

BradReesWork · 2023-05-23T16:11:32Z

the top level folder for python code should be called "python" and then all the moudules under that

BradReesWork · 2023-05-23T15:47:00Z

.gitignore

-Debug
-build/
-
-## Eclipse IDE


you should keep the IDA and Jupyter ignores since many developers use those tools

.gitignore updated.

BradReesWork · 2023-05-23T15:49:19Z

.gitlab-ci.yml

@@ -0,0 +1,103 @@
+# This file is a template, and might need editing before it works on your project.


Is GitLab CI file stil needed?

Thanks for pointing this out. It is not needed and deleted.

BradReesWork · 2023-05-23T15:53:10Z

README.md

-### Examples
-
-Checkout [GNN example](docs/GNNExample.md) for more details. 
+# rapids-wholegraph


Should have a minimal introduction

BradReesWork · 2023-05-23T15:56:15Z

build.sh

@@ -0,0 +1,199 @@
+#!/bin/bash


./build.sh without any arguments does nothing. Is that the intended function?

BradReesWork · 2023-05-23T16:04:22Z

build.sh

@@ -0,0 +1,199 @@
+#!/bin/bash
+


./build.sh pylibwholegraph
Building for ALL supported GPU architectures...
env: ‘setup.py’: No such file or directory

Added ${REPODIR} to setup.py and fixed this issue

seunghwak · 2023-05-23T21:30:06Z

cpp/src/logger.hpp

You may consider using raft's logging mechanism (e.g. https://github.com/rapidsai/raft/blob/branch-23.06/cpp/include/raft/core/logger-macros.hpp).

…using package; 2. add short README.md; 3. fix build.sh to find setup.py

dongxuy04 · 2023-05-26T08:13:28Z

the "cmake" folder should be under the cpp directory

…re fork

dongxuy04 added 2 commits April 14, 2023 09:33

quick fix for CUDA 12

9cc67d9

wholegraph code refactor first PR and RAPIDS CI

d30bd8f

dongxuy04 requested a review from BradReesWork May 10, 2023 06:27

BradReesWork requested review from rlratzel and seunghwak May 10, 2023 13:28

BradReesWork added breaking Introduces a breaking change improvement Improves an existing functionality labels May 10, 2023

added conda environment yml file

0868ab6

renamed build_component.sh to build.sh and add cuGraphOps GATConv

22b18c9

BradReesWork requested review from MatthiasKohl and stadlmax May 12, 2023 13:26

stadlmax reviewed May 12, 2023

View reviewed changes

BradReesWork added 2 commits May 12, 2023 10:33

GitHub Action files

c758616

Merge branch 'branch-23.06' of https://github.com/dongxuy04/wholegraph …

f1d4029

…into branch-23.06

remove duplicated GNN ops and kernels

469ebb8

BradReesWork suggested changes May 23, 2023

View reviewed changes

seunghwak reviewed May 23, 2023

View reviewed changes

add cpp ext option for pytorch and fix some github issues: 1. pytest …

cac6796

…using package; 2. add short README.md; 3. fix build.sh to find setup.py

dongxuy04 closed this May 26, 2023

remove .gitlab-ci.yml

c4734a3

dongxuy04 reopened this May 26, 2023

move torch.utils.cpp_extension inside compile method to fix load befo…

ab5473e

…re fork

BradReesWork changed the base branch from branch-23.06 to refactoring May 26, 2023 13:48

BradReesWork merged commit e8350ba into rapidsai:refactoring May 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First PR for WholeGraph 23.06 refactor and add RAPIDS CI scripts #16

First PR for WholeGraph 23.06 refactor and add RAPIDS CI scripts #16

dongxuy04 commented May 10, 2023

BradReesWork commented May 10, 2023

dongxuy04 commented May 10, 2023

seunghwak commented May 10, 2023 •

edited

Loading

dongxuy04 commented May 10, 2023

BradReesWork commented May 11, 2023

dongxuy04 commented May 12, 2023

MatthiasKohl commented May 12, 2023

stadlmax May 12, 2023

MatthiasKohl commented May 12, 2023

MatthiasKohl commented May 12, 2023

teju85 commented May 12, 2023

dongxuy04 commented May 12, 2023

dongxuy04 commented May 19, 2023

BradReesWork commented May 23, 2023

BradReesWork commented May 23, 2023

BradReesWork May 23, 2023

dongxuy04 May 26, 2023

BradReesWork May 23, 2023

dongxuy04 May 26, 2023

BradReesWork May 23, 2023

BradReesWork May 23, 2023

BradReesWork May 23, 2023

dongxuy04 May 26, 2023

seunghwak May 23, 2023

dongxuy04 commented May 26, 2023

		@@ -0,0 +1,103 @@
		# This file is a template, and might need editing before it works on your project.

First PR for WholeGraph 23.06 refactor and add RAPIDS CI scripts #16

First PR for WholeGraph 23.06 refactor and add RAPIDS CI scripts #16

Conversation

dongxuy04 commented May 10, 2023

BradReesWork commented May 10, 2023

dongxuy04 commented May 10, 2023

seunghwak commented May 10, 2023 • edited Loading

dongxuy04 commented May 10, 2023

BradReesWork commented May 11, 2023

dongxuy04 commented May 12, 2023

MatthiasKohl commented May 12, 2023

Choose a reason for hiding this comment

MatthiasKohl commented May 12, 2023

MatthiasKohl commented May 12, 2023

teju85 commented May 12, 2023

dongxuy04 commented May 12, 2023

dongxuy04 commented May 19, 2023

BradReesWork commented May 23, 2023

BradReesWork commented May 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongxuy04 commented May 26, 2023

seunghwak commented May 10, 2023 •

edited

Loading