Refactoring into 23.08 #24

BradReesWork · 2023-06-09T14:53:00Z

This branch is a collection of all the refactoring improvements.

closes #13
closes #14
closes #17
closes #18
closes https://github.com/rapidsai/cugraph-ops/issues/492

…into branch-23.06

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

…re fork

First PR for WholeGraph 23.06 refactor and add RAPIDS CI scripts

…ta type

Co-authored-by: dongxuy04 <78518666+dongxuy04@users.noreply.github.com>

refactor random genertor cpu

Refactoring and fixing build

Authors: - Brad Rees (https://github.com/BradReesWork) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) URL: #31

.github/ops-bot.yml

Authors: - Brad Rees (https://github.com/BradReesWork) Approvers: - Rick Ratzel (https://github.com/rlratzel) URL: #32

…memory (#34) Try to fix CI. - Use System V API for host shared memory, so that we don't rely on /dev/shm size. Depending on /dev/shm size need `--shm-size` flag at container startup, for RAPIDS CI this may need support in shared-action-workflows. - Enabled all cpp tests. - Fixed append_unique_tests and wholememory_comm_tests in cpp tests. - Enabled all Python tests except for aarch64. Seems there is no public pytorch package avaiable supporting CUDA for aarch64. Authors: - https://github.com/dongxuy04 Approvers: - Brad Rees (https://github.com/BradReesWork) URL: #34

ajschmidt8

left a few comments.

additionally, this repository will also need an update-version.sh script to ensure all of the RAPIDS version numbers stay up-to-date.

see the cugraph file below for reference:

https://github.com/rapidsai/cudf/blob/branch-23.08/ci/release/update-version.sh

ci/build_docs.sh

.github/workflows/build.yaml

.github/workflows/pr.yaml

.github/workflows/test.yaml

ci/test_cpp.sh

ci/test_python.sh

dependencies.yaml

creating docs Authors: - Brad Rees (https://github.com/BradReesWork) Approvers: - https://github.com/dongxuy04 - Don Acosta (https://github.com/acostadon) URL: #35

Updated docs - Added more comments in codes - Merged #35 and enabled docs - Added some MarkDown docs Authors: - https://github.com/dongxuy04 Approvers: - Brad Rees (https://github.com/BradReesWork) - Don Acosta (https://github.com/acostadon) URL: #36

Authors: - Brad Rees (https://github.com/BradReesWork) Approvers: - Don Acosta (https://github.com/acostadon) URL: #39

conda/recipes/libwholegraph/meta.yaml

ajschmidt8

Looks good. In addition the the PR comments, I have one other question/request.

Are all of the Doxygen files needed? I see three right now:

cpp/Doxyfile.in
cpp/Doxyfile
cpp/doxygen/Doxyfile

If not, can you remove the extra ones?

Also, in other repositories, we've intentionally removed CMake as a dependency for building Doxygen docs (see here: rapidsai/cuml#5155).

Can you also do that for this repository? If cpp/Doxyfile.in isn't used, then you'll just need to delete that file to complete this request. That's the only Doxygen file that I see which still uses CMake, though I don't see that file being used anywhere.

ci/build_docs.sh

ci/check_style.sh

ci/test_cpp.sh

ci/release/update-version.sh

As `libcugraphops` is only needed to run GNN applications, not required by `libwholgraph` or `pylibwholegraph`, just remove it from dependency. Updated YAML files to support CUDA 12 CI. Fix exit code in `ci/test_python.sh` and also skip python test for CUDA 12 as no public stable pytorch package. Authors: - https://github.com/dongxuy04 Approvers: - Tingyu Wang (https://github.com/tingyu66) - Brad Rees (https://github.com/BradReesWork) URL: #41

Authors: - Tingyu Wang (https://github.com/tingyu66) Approvers: - Brad Rees (https://github.com/BradReesWork) URL: #40

dongxuy04 · 2023-07-25T14:42:09Z

Looks good. In addition the the PR comments, I have one other question/request.

Are all of the Doxygen files needed? I see three right now:

cpp/Doxyfile.in

cpp/Doxyfile

cpp/doxygen/Doxyfile

If not, can you remove the extra ones?

Also, in other repositories, we've intentionally removed CMake as a dependency for building Doxygen docs (see here: rapidsai/cuml#5155).

Can you also do that for this repository? If cpp/Doxyfile.in isn't used, then you'll just need to delete that file to complete this request. That's the only Doxygen file that I see which still uses CMake, though I don't see that file being used anywhere.

Thanks for helping figure this out, removed cpp/Doxyfile.in and cpp/doxygen/Doxyfile, only cpp/Doxyfile left now.

ajschmidt8 · 2023-07-26T18:35:34Z

I have one more round of suggestions. I had to play with some of this locally, so it's easier for me to just post the patch below and elaborate on the changes.

Details

diff --git a/ci/release/update-version.sh b/ci/release/update-version.sh
index ae90826..eb0720b 100755
--- a/ci/release/update-version.sh
+++ b/ci/release/update-version.sh
@@ -46,10 +46,10 @@ sed_runner 's/'"branch-.*\/RAPIDS.cmake"'/'"branch-${NEXT_SHORT_TAG}\/RAPIDS.cma
 sed_runner 's/'"RAPIDS_VERSION \".*\")"'/'"RAPIDS_VERSION \"${NEXT_SHORT_TAG}\")"'/g' cpp/CMakeLists.txt
 
 # Python CMakeLists updates
-sed_runner 's/'"RAPIDS_VERSION .*)"'/'"RAPIDS_VERSION ${NEXT_FULL_TAG})"'/g' python/pylibwholegraph/CMakeLists.txt
+sed_runner '/set(RAPIDS_VERSION/ s/".*"/'\"${NEXT_SHORT_TAG}\"'/g' python/pylibwholegraph/CMakeLists.txt
 
 # Python setup.py updates
-sed_runner 's/'version=".*"'/'version="${NEXT_FULL_TAG}"'/g' python/pylibwholegraph/setup.py
+sed_runner 's/'version=\".*\"'/'version=\"${NEXT_FULL_TAG}\"'/g' python/pylibwholegraph/setup.py
 
 
 # RTD update
@@ -85,7 +85,7 @@ for DEP in "${DEPENDENCIES[@]}"; do
 done
 
 # Doxyfile update
-sed_runner "s|PROJECT_NUMBER[[:space:]]*=.*|PROJECT_NUMBER=${NEXT_SHORT_TAG}|" cpp/Doxyfile
+sed_runner "/^PROJECT_NUMBER/ s|=.*|= ${NEXT_SHORT_TAG}|" cpp/Doxyfile
 
 
 # CI files
diff --git a/conda/environments/all_cuda-115_arch-x86_64.yaml b/conda/environments/all_cuda-115_arch-x86_64.yaml
index bbadcc0..f163b45 100644
--- a/conda/environments/all_cuda-115_arch-x86_64.yaml
+++ b/conda/environments/all_cuda-115_arch-x86_64.yaml
@@ -23,8 +23,8 @@ dependencies:
 - graphviz
 - ipykernel
 - ipython
-- libraft-headers==23.08.*
-- librmm==23.08.*
+- libraft-headers==23.8.*
+- librmm==23.8.*
 - nanobind>=0.2.0
 - nbsphinx
 - nccl
diff --git a/conda/environments/all_cuda-118_arch-x86_64.yaml b/conda/environments/all_cuda-118_arch-x86_64.yaml
index 70e3721..ec637bb 100644
--- a/conda/environments/all_cuda-118_arch-x86_64.yaml
+++ b/conda/environments/all_cuda-118_arch-x86_64.yaml
@@ -23,8 +23,8 @@ dependencies:
 - graphviz
 - ipykernel
 - ipython
-- libraft-headers==23.08.*
-- librmm==23.08.*
+- libraft-headers==23.8.*
+- librmm==23.8.*
 - nanobind>=0.2.0
 - nbsphinx
 - nccl
diff --git a/conda/environments/all_cuda-120_arch-x86_64.yaml b/conda/environments/all_cuda-120_arch-x86_64.yaml
index b354691..b532944 100644
--- a/conda/environments/all_cuda-120_arch-x86_64.yaml
+++ b/conda/environments/all_cuda-120_arch-x86_64.yaml
@@ -25,8 +25,8 @@ dependencies:
 - graphviz
 - ipykernel
 - ipython
-- libraft-headers==23.08.*
-- librmm==23.08.*
+- libraft-headers==23.8.*
+- librmm==23.8.*
 - nanobind>=0.2.0
 - nbsphinx
 - nccl
diff --git a/dependencies.yaml b/dependencies.yaml
index 280aae0..2eb6d63 100644
--- a/dependencies.yaml
+++ b/dependencies.yaml
@@ -164,8 +164,8 @@ dependencies:
     common:
       - output_types: [conda, requirements]
         packages:
-          - libraft-headers==23.08.*
-          - librmm==23.08.*
+          - libraft-headers==23.8.*
+          - librmm==23.8.*
   test_cpp:
     common:
       - output_types: [conda, requirements]

The changes are as follows:

update-version.sh:
- some of the suggestions here just fix some quotes that weren't working correctly
- other suggestions make use of sed addresses to fix and simplify the search and replace suggestions. sed addresses work by specifying a range of line(s) that the subsequent search and replace expression should be run on. so sed /^THE_LINE/ s/E/S/ would only run the s/E/S operation on lines that start with THE_LINE
version specifiers - I've updated the version specifiers in both dependencies.yaml and the environment files it generates to follow PEP 440 standards. though it's not formally recorded anywhere, this is the convention we've adopted for other repos (like cudf here) so that our version specifiers can be compatible with both conda and pip.

The result of the update-version.sh changes is that you can run the following sequence of commands and the net result will be no changes, which is the ideal behavior:

bash ci/release/update-version.sh 23.08.00
bash ci/release/update-version.sh 23.10.00
bash ci/release/update-version.sh 24.08.00
bash ci/release/update-version.sh 23.08.00 # back to `23.08.00`. no net changes

Clean up scripts for packaging and revise conda recipes. Authors: - Tingyu Wang (https://github.com/tingyu66) Approvers: - Brad Rees (https://github.com/BradReesWork) - AJ Schmidt (https://github.com/ajschmidt8) URL: #42

tingyu66

Conda recipes and wheel building script LGTM.

tingyu66 · 2023-07-28T02:09:09Z

I have one more round of suggestions. I had to play with some of this locally, so it's easier for me to just post the patch below and elaborate on the changes.

Details
The changes are as follows:

update-version.sh:

some of the suggestions here just fix some quotes that weren't working correctly

other suggestions make use of sed addresses to fix and simplify the search and replace suggestions. sed addresses work by specifying a range of line(s) that the subsequent search and replace expression should be run on. so sed /^THE_LINE/ s/E/S/ would only run the s/E/S operation on lines that start with THE_LINE

version specifiers - I've updated the version specifiers in both dependencies.yaml and the environment files it generates to follow PEP 440 standards. though it's not formally recorded anywhere, this is the convention we've adopted for other repos (like cudf here) so that our version specifiers can be compatible with both conda and pip.

The result of the update-version.sh changes is that you can run the following sequence of commands and the net result will be no changes, which is the ideal behavior:
bash ci/release/update-version.sh 23.08.00
bash ci/release/update-version.sh 23.10.00
bash ci/release/update-version.sh 24.08.00
bash ci/release/update-version.sh 23.08.00 # back to `23.08.00`. no net changes

Thanks for the suggestions. They should be addressed in #44. Please take a look over there since I cannot request your review due to priviledges.

dongxuy04

LGTM, thanks!

ajschmidt8

looks good. two small comments left.

.pre-commit-config.yaml

ci/build_docs.sh

ajschmidt8 · 2023-07-28T20:10:17Z

/merge

dongxuy04 and others added 22 commits April 14, 2023 09:33

quick fix for CUDA 12

9cc67d9

wholegraph code refactor first PR and RAPIDS CI

d30bd8f

added conda environment yml file

0868ab6

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

22b18c9

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

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

remove .gitlab-ci.yml

c4734a3

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

ab5473e

…re fork

Merge pull request #16 from dongxuy04/branch-23.06

e8350ba

First PR for WholeGraph 23.06 refactor and add RAPIDS CI scripts

refactor random genertor cpu

81fbac1

rodo file structure

c565b66

rename raft_random_gen.cpp to raft_random_gen.cu and modified some da…

f8af2dd

…ta type

adjusted cmake calls to new directory level

d896c9c

updates

b6225ee

Update build.sh

1988835

Co-authored-by: dongxuy04 <78518666+dongxuy04@users.noreply.github.com>

Merge pull request #20 from hello-11/refactor_random_generator_op

d39454a

refactor random genertor cpu

fixed errors and removed bedug lines

f590e6c

Merge branch 'rapidsai:branch-23.08' into refactor_fix_1

a4900b0

update conflicts

49eea8a

Merge pull request #22 from BradReesWork/refactor_fix_1

4ad632e

Refactoring and fixing build

BradReesWork requested review from dongxuy04, rlratzel, MatthiasKohl and stadlmax June 9, 2023 14:53

BradReesWork added breaking Introduces a breaking change improvement Improves an existing functionality labels Jun 9, 2023

BradReesWork added this to the 23.08 milestone Jun 9, 2023

fix codeowners

427c4a4

BradReesWork reopened this Jun 20, 2023

Rename ops-bot.yaml to ops-bot.yml (#31)

a8a9990

Authors: - Brad Rees (https://github.com/BradReesWork) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) URL: #31

ajschmidt8 reviewed Jun 20, 2023

View reviewed changes

.github/ops-bot.yml Outdated Show resolved Hide resolved

BradReesWork and others added 3 commits June 23, 2023 01:06

Refactoring ci fixes (#32)

2033505

Authors: - Brad Rees (https://github.com/BradReesWork) Approvers: - Rick Ratzel (https://github.com/rlratzel) URL: #32

Delete ops-bot.yaml

b45b4f1

ajschmidt8 requested changes Jun 29, 2023

View reviewed changes

Adding Docs (#35)

d058288

creating docs Authors: - Brad Rees (https://github.com/BradReesWork) Approvers: - https://github.com/dongxuy04 - Don Acosta (https://github.com/acostadon) URL: #35

BradReesWork linked an issue Jul 10, 2023 that may be closed by this pull request

Refactor code #14

Closed

dongxuy04 and others added 2 commits July 12, 2023 14:01

Update docs (#36)

6a758cf

Updated docs - Added more comments in codes - Merged #35 and enabled docs - Added some MarkDown docs Authors: - https://github.com/dongxuy04 Approvers: - Brad Rees (https://github.com/BradReesWork) - Don Acosta (https://github.com/acostadon) URL: #36

addressing comments (#39)

8275eb6

Authors: - Brad Rees (https://github.com/BradReesWork) Approvers: - Don Acosta (https://github.com/acostadon) URL: #39

tingyu66 reviewed Jul 20, 2023

View reviewed changes

conda/recipes/libwholegraph/meta.yaml Outdated Show resolved Hide resolved

ajschmidt8 requested changes Jul 24, 2023

View reviewed changes

dongxuy04 and others added 2 commits July 25, 2023 14:14

Build wheels for pylibwholegraph (#40)

e5168c5

Authors: - Tingyu Wang (https://github.com/tingyu66) Approvers: - Brad Rees (https://github.com/BradReesWork) URL: #40

Clean up scripts for packaging (#42)

73471f3

Clean up scripts for packaging and revise conda recipes. Authors: - Tingyu Wang (https://github.com/tingyu66) Approvers: - Brad Rees (https://github.com/BradReesWork) - AJ Schmidt (https://github.com/ajschmidt8) URL: #42

tingyu66 approved these changes Jul 27, 2023

View reviewed changes

tingyu66 mentioned this pull request Jul 28, 2023

Revise update-version.sh #44

Merged

dongxuy04 approved these changes Jul 28, 2023

View reviewed changes

Revise update-version.sh (#44)

6d73391

ajschmidt8 reviewed Jul 28, 2023

View reviewed changes

.pre-commit-config.yaml Outdated Show resolved Hide resolved

ci/build_docs.sh Outdated Show resolved Hide resolved

tingyu66 mentioned this pull request Jul 28, 2023

address comments #45

Merged

address comments (#45)

8985989

ajschmidt8 approved these changes Jul 28, 2023

View reviewed changes

rapids-bot bot merged commit aba3fd7 into branch-23.08 Jul 28, 2023
39 checks passed

dongxuy04 mentioned this pull request Aug 3, 2023

Usage of C10 API's in WholeMemory and its implication on build #6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactoring into 23.08 #24

Refactoring into 23.08 #24

BradReesWork commented Jun 9, 2023 •

edited

Loading

ajschmidt8 left a comment

ajschmidt8 left a comment

dongxuy04 commented Jul 25, 2023

ajschmidt8 commented Jul 26, 2023

tingyu66 left a comment

tingyu66 commented Jul 28, 2023

dongxuy04 left a comment

ajschmidt8 left a comment

ajschmidt8 commented Jul 28, 2023

Refactoring into 23.08 #24

Refactoring into 23.08 #24

Conversation

BradReesWork commented Jun 9, 2023 • edited Loading

ajschmidt8 left a comment

Choose a reason for hiding this comment

ajschmidt8 left a comment

Choose a reason for hiding this comment

dongxuy04 commented Jul 25, 2023

ajschmidt8 commented Jul 26, 2023

tingyu66 left a comment

Choose a reason for hiding this comment

tingyu66 commented Jul 28, 2023

dongxuy04 left a comment

Choose a reason for hiding this comment

ajschmidt8 left a comment

Choose a reason for hiding this comment

ajschmidt8 commented Jul 28, 2023

BradReesWork commented Jun 9, 2023 •

edited

Loading