Add Khop Graph Sampler API #39146
Add Khop Graph Sampler API #39146
Conversation
…ti_device_sample
… multi_device_sample
…ti_device_sample
… debug_graph_sample
|
Thanks for your contribution! |
| } | ||
|
|
||
| ctx->SetOutputDim("Out_Src", {-1, 1}); | ||
| ctx->SetOutputDim("Out_Dst", {-1, 1}); |
There was a problem hiding this comment.
有个疑问，咱们的输出的out_src, out_dst一定需要是二维的吗？需要和PGL目前的版本一致吗？
| auto unique_inputs_end = std::unique(inputs.begin(), inputs.end()); | ||
| inputs.resize(std::distance(inputs.begin(), unique_inputs_end)); | ||
|
|
||
| // 2. Sample neighbors. |
|
|
||
| template <class bidiiter> | ||
| void sample_unique(bidiiter begin, bidiiter end, int num_samples) { | ||
| size_t left = std::distance(begin, end); |
There was a problem hiding this comment.
这里的变量名字有点奇怪，叫left比较奇怪，是想说剩下的元素的个数吗？
There was a problem hiding this comment.
是的，因为这个变量在for 循环中是需要不断减一的。这里我修改成 left_num。
| template <class bidiiter> | ||
| void sample_unique(bidiiter begin, bidiiter end, int num_samples) { | ||
| size_t left = std::distance(begin, end); | ||
| unsigned int seed = left; |
There was a problem hiding this comment.
这块的逻辑没有问题， 但是一个signed的变量转化成一个unsigned多少有点风险，后面可能需要注意这块的转化的问题
There was a problem hiding this comment.
已删除这个地方的转换，换用另一种方式产生
| auto unique_dst_merge_ptr = unique_dst_merge.begin(); | ||
| auto src_merge_ptr = src_merge.begin(); | ||
| auto dst_sample_counts_merge_ptr = dst_sample_counts_merge.begin(); | ||
| for (size_t i = 0; i < num_layers; i++) { |
There was a problem hiding this comment.
后续的Copy逻辑需要再看看，记个TODO, 看看能不能通过std::move减少一些copy
| auto* out_eids = ctx.Output<Tensor>("Out_Eids"); | ||
| out_eids->Resize({static_cast<int>(eids_merge.size())}); | ||
| T* p_out_eids = out_eids->mutable_data<T>(ctx.GetPlace()); | ||
| memset(p_out_eids, 0, eids_merge.size() * sizeof(T)); |
There was a problem hiding this comment.
这里的memset看看是否多余，因为后面就是一个直接Copy的操作
| dst_cumsum_counts, nodes, | ||
| "sample_sizes", sample_sizes, | ||
| "return_eids", False) | ||
| return edge_src, edge_dst, sample_index, reindex_nodes |
There was a problem hiding this comment.
这里有个想法就是把sorted_eids 默认为None, 这样的化就可以解决你下面的问题
| nodes, | ||
| sample_sizes, | ||
| return_eids=False, | ||
| name=None): |
| class GraphSampleNeighborsOpCUDAKernel : public framework::OpKernel<T> { | ||
| public: | ||
| void Compute(const framework::ExecutionContext& ctx) const override { | ||
| // 1. Get inputs. |
| } | ||
|
|
||
| template <typename T> | ||
| void sample_neighbors(const framework::ExecutionContext& ctx, const T* src, |
There was a problem hiding this comment.
函数名 需要符合一下规范 2.函数 普通函数:以大写字母开头,每个单词首字母大写,无下划线。 AddTabEntry() DeleteUrl() 存取函数:要求与变量名匹配(TODO)
| } | ||
|
|
||
| template <typename T> | ||
| void reindex_func(const framework::ExecutionContext& ctx, |
| thrust::copy(unique_items.begin(), unique_items.end(), subset->begin()); | ||
|
|
||
| // Fill outputs with reindex result. | ||
| int block = 1024; |
| int block = 1024; | ||
| int grid = (outputs->size() + block - 1) / block; | ||
| reindex_src_output< | ||
| T><<<grid, block, 0, reinterpret_cast<const platform::CUDADeviceContext&>( |
There was a problem hiding this comment.
reinterpret_cast 这块不是应该是函数一个指针吗 为啥是引用
paddle/fluid/pybind/imperative.cc
Outdated
| } | ||
| return new_tensor; | ||
| }, | ||
| py::return_value_policy::reference, R"DOC()DOC"); |
… debug_graph_sample
…evice_id, fix bug for input nodes with empty neighbors
PR types
New features
PR changes
APIs
Describe