Replace raw pointers with device_span in induced subgraph by yang-hu-nv · Pull Request #2348 · rapidsai/cugraph

yang-hu-nv · 2022-06-07T22:21:24Z

Changed the interface of function extract_induced_subgraphs, and all corresponding test codes using this interface.

GPUtester · 2022-06-07T22:21:26Z

Can one of the admins verify this patch?

GPUtester · 2022-06-07T22:21:26Z

Can one of the admins verify this patch?

seunghwak · 2022-06-07T22:26:54Z

This PR updates the public API of induced_subgrpah, so breaking.

seunghwak · 2022-06-07T22:27:41Z

cpp/src/structure/induced_subgraph_mg.cu

-                          size_t const* subgraph_offsets,
-                          int32_t const* subgraph_vertices,
+                          raft::device_span<size_t const> subgraph_offsets /*size_t const* subgraph_offsets*/,
+                          raft::device_span<int_32 const> subgraph_vertices /*int32_t const* subgraph_vertices*/,


I assume int_32 here should be int32_t

seunghwak · 2022-06-18T05:39:40Z

rerun tests

cpp/src/community/legacy/egonet.cu

seunghwak · 2022-06-18T06:24:10Z

cpp/src/community/legacy/egonet.cu

-  return cugraph::extract_induced_subgraphs(
-    handle, csr_view, neighbors_offsets.data().get(), neighbors.data().get(), n_subgraphs);
+  //return cugraph::extract_induced_subgraphs(
+    //handle, csr_view, dspan_neighbors_offsets, dspan_neighbors, n_subgraphs, false);


Why did you commented this out?

I had to comment out these lines to finish the compiling. And the EGO_TEST does not work now.

There are errors on line 171 when I construct raft::device_span<size_t const> d_neighbors_offsets and raft::device_span<vertex_t const> d_neighbors, no matter I use neighbors_offsets.data() or neighbors_offsets.data().get() as the input pointer.

error message:
/home/nfs/yangh/cugraph/cpp/src/community/legacy/egonet.cu(171): error: a pointer to a bound function may only be used to call the function
detected during instantiation of "std::tuple<rmm::device_uvector<vertex_t>, rmm::device_uvector<vertex_t>, std::optional<rmm::device_uvector<weight_t>>, rmm::device_uvector<size_t>> cugraph::extract_ego(const raft::handle_t &, const cugraph::graph_view_t<vertex_t, edge_t, weight_t, false, multi_gpu, void> &, vertex_t *, vertex_t, vertex_t) [with vertex_t=int32_t, edge_t=int32_t, weight_t=float, multi_gpu=false]"

return cugraph::extract_induced_subgraphs(handle, csr_view, raft::device_span<size_t const>(neighbor_offsets.data().get(), neighbor_offsets.size()), raft::device_span<vertex_t const>(neighbors.data().get(), neighbors.size(), n_subgraphs, false);

cpp/src/structure/induced_subgraph_impl.cuh

cpp/include/cugraph/graph_functions.hpp

cpp/src/structure/induced_subgraph_impl.cuh

cpp/src/structure/induced_subgraph_mg.cu

cpp/tests/community/egonet_test.cu

cpp/tests/community/induced_subgraph_test.cpp

huyang1988 · 2022-06-21T21:15:14Z

cpp/src/structure/induced_subgraph_impl.cuh

-          subgraph_offsets + 1,
-          thrust::upper_bound(thrust::seq, subgraph_offsets, subgraph_offsets + num_subgraphs, i));
+          subgraph_offsets.begin()+1, 
+          thrust::upper_bound(thrust::seq, subgraph_offsets.begin(), subgraph_offsets.end()-1, i));


Need change .end()-1 to .data() + num_subgraphs

thrust::upper_bound works with iterators so, you can use either begin() and end() - 1 or .data() and .data() + num_subhgraphs (but shouldn't mix begin() and data() + num_subgraphs).

So, I don't see a need to replace end() - 1 to data() + num_subgraphs.,

ChuckHastings

LGTM

ChuckHastings · 2022-06-21T21:45:28Z

rerun tests

rlratzel · 2022-06-21T22:08:28Z

add to allowlist

seunghwak · 2022-06-21T22:12:24Z

@yang-hu-nv
If you click Details after gpuCI/cugraph/check/style — Build #4950 failed in 25 sec, you can see

>>>> FAILED: clang format check; begin output





graph_functions.hpp failed! 'diff cpp/include/cugraph/graph_functions.hpp /tmp/tmpf13b78nz/cpp/include/cugraph/graph_functions.hpp' will show formatting violations!

induced_subgraph_impl.cuh failed! 'diff cpp/src/structure/induced_subgraph_impl.cuh /tmp/tmpf13b78nz/cpp/src/structure/induced_subgraph_impl.cuh' will show formatting violations!

induced_subgraph_mg.cu failed! 'diff cpp/src/structure/induced_subgraph_mg.cu /tmp/tmpf13b78nz/cpp/src/structure/induced_subgraph_mg.cu' will show formatting violations!

egonet.cu failed! 'diff cpp/src/community/legacy/egonet.cu /tmp/tmpf13b78nz/cpp/src/community/legacy/egonet.cu' will show formatting violations!

induced_subgraph_test.cpp failed! 'diff cpp/tests/community/induced_subgraph_test.cpp /tmp/tmpf13b78nz/cpp/tests/community/induced_subgraph_test.cpp' will show formatting violations!

sg_uniform_neighbor_sampling.cu failed! 'diff cpp/tests/sampling/sg_uniform_neighbor_sampling.cu /tmp/tmpf13b78nz/cpp/tests/sampling/sg_uniform_neighbor_sampling.cu' will show formatting violations!

clang-format failed! You have 2 options:

 1. Look at formatting differences above and fix them manually

 2. Or run the below command to bulk-fix all these at once

Bulk-fix command: 

  python cpp/scripts/run-clang-format.py  -inplace





>>>> FAILED: clang format check; end output

You need to run clang-format for the files mentioned above to address this.

seunghwak · 2022-06-22T00:36:43Z

>>>> FAILED: copyright check; begin output



Switched to a new branch 'branch-22.08'

Switched to branch 'current-pr-branch'

Switched to branch 'current-pr-branch'

   [DEBUG] TARGET_BRANCH=branch-22.08, COMMIT_HASH=origin/pr/2348/merge, currentBranch=current-pr-branch

   [DEBUG] Assuming a CI environment.

   [DEBUG] Found files to check:

	cpp/include/cugraph/graph_functions.hpp

	cpp/src/community/legacy/egonet.cu

	cpp/src/structure/induced_subgraph_impl.cuh

	cpp/src/structure/induced_subgraph_mg.cu

	cpp/src/structure/induced_subgraph_sg.cu

	cpp/tests/community/induced_subgraph_test.cpp

	cpp/tests/sampling/sg_uniform_neighbor_sampling.cu


Copyright headers incomplete in some of the files!

  cpp/src/structure/induced_subgraph_mg.cu:2 Issue: Current year not included in the copyright header

  cpp/src/structure/induced_subgraph_sg.cu:2 Issue: Current year not included in the copyright header


You can run `python ci/checks/copyright.py --git-modified-only --update-current-year` to fix 2 of these errors.



>>>> FAILED: copyright check; end output

so, if you have updated a file, the copyright statement should be updated to include the current year as well.

seunghwak

LGTM!!!

ChuckHastings · 2022-06-22T19:17:50Z

@gpucibot merge

~

1ae1c99

yang-hu-nv requested a review from a team as a code owner June 7, 2022 22:21

yang-hu-nv closed this Jun 7, 2022

yang-hu-nv reopened this Jun 7, 2022

yang-hu-nv changed the title ~~test PR~~ [WIP][skip-ci] replace raw pointers with device_span in induced subgraph Jun 7, 2022

seunghwak assigned yang-hu-nv Jun 7, 2022

seunghwak added 2 - In Progress improvement Improvement / enhancement to an existing function breaking Breaking change labels Jun 7, 2022

seunghwak added this to the 22.08 milestone Jun 7, 2022

seunghwak reviewed Jun 7, 2022

View reviewed changes

seunghwak added the good first issue label Jun 7, 2022

fea_mg_induced_subgraph

b7a8aee

seunghwak changed the title ~~[WIP][skip-ci] replace raw pointers with device_span in induced subgraph~~ Replace raw pointers with device_span in induced subgraph Jun 18, 2022

seunghwak reviewed Jun 18, 2022

View reviewed changes

yang-hu-nv added 2 commits June 18, 2022 10:44

fix problems

d7c8f6d

wrap up

b0d4a32

seunghwak added 3 - Ready for Review and removed 2 - In Progress labels Jun 21, 2022

huyang1988 reviewed Jun 21, 2022

View reviewed changes

seunghwak requested a review from ChuckHastings June 21, 2022 21:20

ChuckHastings approved these changes Jun 21, 2022

View reviewed changes

clang-format check

9cf14ad

copyright check

92667c1

seunghwak approved these changes Jun 22, 2022

View reviewed changes

rapids-bot bot merged commit b4a4160 into rapidsai:branch-22.08 Jun 22, 2022

Conversation

yang-hu-nv commented Jun 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GPUtester commented Jun 7, 2022

Uh oh!

GPUtester commented Jun 7, 2022

Uh oh!

seunghwak commented Jun 7, 2022

Uh oh!

seunghwak Jun 7, 2022

Choose a reason for hiding this comment

Uh oh!

seunghwak commented Jun 18, 2022

Uh oh!

Uh oh!

Uh oh!

seunghwak Jun 18, 2022

Choose a reason for hiding this comment

Uh oh!

yang-hu-nv Jun 18, 2022

Choose a reason for hiding this comment

Uh oh!

seunghwak Jun 21, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

huyang1988 Jun 21, 2022

Choose a reason for hiding this comment

Uh oh!

seunghwak Jun 21, 2022

Choose a reason for hiding this comment

Uh oh!

seunghwak Jun 21, 2022

Choose a reason for hiding this comment

Uh oh!

ChuckHastings left a comment

Choose a reason for hiding this comment

Uh oh!

ChuckHastings commented Jun 21, 2022

Uh oh!

rlratzel commented Jun 21, 2022

Uh oh!

seunghwak commented Jun 21, 2022

Uh oh!

seunghwak commented Jun 22, 2022

Uh oh!

seunghwak left a comment

Choose a reason for hiding this comment

Uh oh!

ChuckHastings commented Jun 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

yang-hu-nv commented Jun 7, 2022 •

edited

Loading