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

Update Sampling API prior to MTMG integration. #4646

Closed

Conversation

ChuckHastings
Copy link
Collaborator

@ChuckHastings ChuckHastings commented Sep 4, 2024

First cut at new sampling function definition to clean up things before MTMG integration.

This overlaps some with @jnke2016 work on heterogeneous sampling... we'll need to resolve a merge order once we've decided on the exact API changes and sequence of work.

Latest thought is that this will be merged into @jnke2016's PR and this will be closed once it's no longer needed.

@github-actions github-actions bot added the CMake label Sep 11, 2024
@ChuckHastings ChuckHastings added the DO NOT MERGE Hold off on merging; see PR for details label Sep 16, 2024
std::optional<rmm::device_uvector<edge_type_t>>,
std::optional<rmm::device_uvector<int32_t>>,
std::optional<rmm::device_uvector<size_t>>>
homogeneous_uniform_neighbor_sample(
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we discussed that the API should have two categories: Heterogeneous_neighbor_sample and Homogeneous_neighbor_sample where each category supports uniform and bias

std::get<0>(label_to_output_comm_rank).begin(),
std::get<0>(label_to_output_comm_rank).end(),
std::get<1>(label_to_output_comm_rank).begin(),
label_map.begin());
Copy link
Contributor

Choose a reason for hiding this comment

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

You mean thrust::scatter?


label_map.resize(max_label, handle.get_stream());

thrust::fill(handle.get_thrust_policy(), label_map.begin(), label_map.end(), label_t{0});
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the fill value should be an int32_t value (not label_t{0}).

@ChuckHastings
Copy link
Collaborator Author

Incorporated into @jnke2016 PR... closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake cuGraph DO NOT MERGE Hold off on merging; see PR for details
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants