-
Notifications
You must be signed in to change notification settings - Fork 310
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
Update Sampling API prior to MTMG integration. #4646
Conversation
…re MTMG integration
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( |
There was a problem hiding this comment.
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()); |
There was a problem hiding this comment.
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}); |
There was a problem hiding this comment.
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}).
Incorporated into @jnke2016 PR... closing this. |
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.