Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP][skip-ci] SCC part 1 - trimming vertices with 0 in or out degrees #3325

Closed
wants to merge 33 commits into from

Conversation

wu6u3
Copy link

@wu6u3 wu6u3 commented Mar 9, 2023

No description provided.

@wu6u3 wu6u3 requested review from a team as code owners March 9, 2023 20:10
@rapids-bot
Copy link

rapids-bot bot commented Mar 9, 2023

Pull requests from external contributors require approval from a rapidsai organization member with write or admin permissions before CI can begin.

@seunghwak seunghwak changed the title Adding SCC tests [WIP][skip-ci] SCC part 1 - trimming vertices with 0 in or out degrees Mar 9, 2023
remaining_vertices.begin(),
[in_degrees] __device__(
auto v) { return out_degrees[v] > edge_t{0}; })),
handle.get_stream());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to trim 0 in|out degree vertices from the graph.

You can refer to https://github.com/rapidsai/cugraph/blob/branch-23.04/cpp/src/community/triangle_count_impl.cuh#L261 (see line 234-298).

You can copy-and-paste-and-modify this code. The return value of this function should be a graph object and a renumber_map. See the link above and let me know if you have questions (code related to renumbering can be confusing if you are new to cuGraph, so don't hesitate to ask me questions).

Also note that extract_if_e will be renamed to extract_transform_e with some changes in function input parameters. See #3323.

graph_view);
auto in_zero_core_first =
thrust::make_transform_iterator(core_numbers.begin(), is_zero_or_greater_t<edge_t>{});
rmm::device_uvector<uint8_t> in_zero_core_flags(core_numbers.size(), handle.get_stream());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you finding 0-core? Vertex degrees can't be negative, so 0-core is the entire graph.

Here, we are trying to extract a subgraph where every vertex has non-zero in&out degree (as zero indegree or out degree vertices are singleton components).

You can do something like the following.

auto in_degrees = graph_view.compute_in_degrees(handle);
auto out_degrees = graph_view.compute_out_degrees(handle);
rmm::device_uvector<uint8_t> include_flags(in_degrees.size(), handle.get_stream());
auto degree_pair_first = thrust::make_zip_iterator(thrust::make_tuple(in_degrees.begin(), out_degrees.begin()));
thrust::transform(handle.get_thrust_policy(), degree_pair_first,  degree_pair_last, []__device__(auto pair) {
  return (thrust::get<0>(pair) >= 1 && thrust::get<1>(pair) >= 1) ? uint8_t{1} : uint8_t{0};
});
edge_src_property_t<decltype(graph_view), uint8_t> edge_src_include_flags(handle, graph_view);
edge_dst_property_t<decltype(graph_view), uint8_t> edge_dst_include_flags(handle, graph_view);
update_edge_src_property(...);
update_edge_dst_property(...);

@BradReesWork BradReesWork added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 28, 2023
@BradReesWork BradReesWork changed the base branch from branch-23.04 to branch-23.06 March 30, 2023 12:43
@BradReesWork BradReesWork added this to the 23.06 milestone Mar 30, 2023
@wu6u3 wu6u3 requested a review from a team as a code owner April 20, 2023 18:44
#include <cugraph/graph_view.hpp>
#include <cugraph/partition_manager.hpp>
#include <cugraph/utilities/error.hpp>
#include <cugraph/vertex_partition_device_view.cuh>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a space between cugraph includes and thrust includes.

#include <thrust/copy.h>
#include <thrust/iterator/discard_iterator.h>


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One empty line is enough between two include groups.

@@ -0,0 +1,186 @@
#pragma once
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#pragma once unnecessary for trim.cu.

@@ -0,0 +1,186 @@
#pragma once
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And add a license statement at the beginning.

/*
 * Copyright (c) 2023, NVIDIA CORPORATION.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governin_from_mtxg permissions and
 * limitations under the License.
 */

std::optional<edge_property_view_t<edge_t, weight_t const*>> edge_weight_view,
bool do_expensive_check)
{

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for an empty line here.

raft::update_device(d_subgraph_offsets_out.data(),
h_subgraph_offsets_out.data(),
h_subgraph_offsets_out.size(),
handle.get_stream());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to extract a subgraph here and create a new graph from the extracted edges.

Comment on lines 99 to 106
auto subgraph_out_view = subgraph_out.view();
auto [src, dst, wgt, offsets] = extract_induced_subgraphs(
handle,
subgraph_out_view,
subgraph_edge_weights_out,
raft::device_span<size_t const>{d_subgraph_offsets_out.data(), d_subgraph_offsets_out.size()},
raft::device_span<vertex_t const>{out_one_core_flags.data(), out_one_core_flags.size()},
do_expensive_check);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're first extracting a one core (based on the out-degree) from the original graph.

Then, you create a new graph (from the edges that belong to the one core).

Then, extract a one core (based on the in-degree) from the new graph.

This code's return value should be based on the original vertex's renumbering, so you unrenumber at this end.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ping me if this is confusing.

minor_ptrs,
major_ptrs.size(),
(*renumber_map_out).data(),
(*renumber_map_out).size());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unrenumber should happen after extracting 1 core.

Comment on lines 153 to 168
std::tie(subgraph_in, subgraph_edge_weights_in, std::ignore, std::ignore, renumber_map_in) =
cugraph::create_graph_from_edgelist<vertex_t,
edge_t,
weight_t,
edge_t,
int32_t,
false,
multi_gpu>(handle, in_one_core_flags, edge_src_in_one_cores, edge_dst_in_one_cores, edge_weight_view);
auto subgraph_in_view = subgraph_in.view();
auto [src_, dst_, wgt_, offsets_] = extract_induced_subgraphs(
handle,
subgraph_in_view,
subgraph_edge_weights_in,
raft::device_span<size_t const>{d_subgraph_offsets_in.data(), d_subgraph_offsets_in.size()},
raft::device_span<vertex_t const>{subgraph_in.data(), subgraph_in.size()},
do_expensive_check);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary.

Comment on lines 122 to 147
core_number(
handle, graph_view, core_numbers.data(), k_core_degree_type_t::IN, size_t{1}, size_t{1});

auto in_one_core_first =
thrust::make_transform_iterator(core_numbers.begin(), is_one_or_greater_t<edge_t>{});
rmm::device_uvector<uint8_t> in_one_core_flags(core_numbers.size(), handle.get_stream());
thrust::copy(handle.get_thrust_policy(),
in_one_core_first,
in_one_core_first + core_numbers.size(),
in_one_core_flags.begin());

edge_src_property_t<decltype(graph_view), uint8_t> edge_src_in_one_cores(handle,
graph_view);
edge_dst_property_t<decltype(graph_view), uint8_t> edge_dst_in_one_cores(handle,
graph_view);

update_edge_src_property(
handle, graph_view, in_one_core_flags.begin(), edge_src_in_one_cores);
update_edge_dst_property(
handle, graph_view, in_one_core_flags.begin(), edge_dst_in_one_cores);
rmm::device_uvector<size_t> d_subgraph_offsets_in(2, handle.get_stream());
std::vector<size_t> h_subgraph_offsets_in{{0, in_one_core_flags.size()}};
raft::update_device(d_subgraph_offsets_in.data(),
h_subgraph_offsets_in.data(),
h_subgraph_offsets_in.size(),
handle.get_stream());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you do all these if you're not calling extract_subgraph to extract 1 (indegree) core?

@ChuckHastings ChuckHastings modified the milestones: 23.06, 23.08 May 23, 2023
@BradReesWork BradReesWork changed the base branch from branch-23.06 to branch-23.08 May 30, 2023 12:00
@BradReesWork BradReesWork modified the milestones: 23.08, 23.10 Jul 24, 2023
@copy-pr-bot
Copy link

copy-pr-bot bot commented Sep 25, 2023

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@BradReesWork BradReesWork changed the base branch from branch-23.08 to branch-23.10 September 25, 2023 14:25
@BradReesWork
Copy link
Member

/okay to test

@ChuckHastings ChuckHastings modified the milestones: 23.10, 23.12 Sep 25, 2023
@BradReesWork BradReesWork changed the base branch from branch-23.10 to branch-23.12 September 26, 2023 13:54
@naimnv naimnv added the DO NOT MERGE Hold off on merging; see PR for details label Oct 20, 2023
@BradReesWork BradReesWork marked this pull request as draft November 6, 2023 21:09
@ChuckHastings ChuckHastings modified the milestones: 23.12, 24.02 Nov 14, 2023
@BradReesWork BradReesWork changed the base branch from branch-23.12 to branch-24.04 January 23, 2024 19:04
@BradReesWork BradReesWork modified the milestones: 24.02, 24.04 Jan 23, 2024
@BradReesWork BradReesWork modified the milestones: 24.04, 24.06 Mar 12, 2024
@BradReesWork BradReesWork changed the base branch from branch-24.04 to branch-24.06 March 19, 2024 17:42
@seunghwak
Copy link
Contributor

This PR won't be merged.

@seunghwak seunghwak closed this Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE Hold off on merging; see PR for details improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants