HITS ported from gunrock/gunrock #83
Conversation
| void print_result(std::ostream& os = std::cout){ | ||
| os<<"===Authority\n\n"; | ||
| for(int i = 0; i < this->auth.size(); i++){ | ||
| os<<"vertex ID: "<<this->auth_vertex[i]<<std::endl; | ||
| os<<"authority: "<<this->auth[i]<<std::endl; | ||
| } | ||
| os<<"===Hub\n\n"; | ||
| for(int i = 0; i < this->hub.size(); i++){ | ||
| os<<"vertex ID: "<<this->hub_vertex[i]<<std::endl; | ||
| os<<"authority: "<<this->hub[i]<<std::endl; | ||
| } | ||
| } | ||
|
|
||
| void print_result(int max_vertices, std::ostream& os = std::cout){ | ||
| int vertices = (max_vertices < this->hub.size()) | ||
| ? max_vertices | ||
| : this->hub.size(); | ||
| os<<"===Authority\n\n"; | ||
| for(int i = 0; i < vertices; i++){ | ||
| os<<"vertex ID: "<<this->auth_vertex[i]<<std::endl; | ||
| os<<"authority: "<<this->auth[i]<<std::endl; | ||
| } | ||
| os<<"===Hub\n\n"; | ||
| for(int i = 0; i < vertices; i++){ | ||
| os<<"vertex ID: "<<this->hub_vertex[i]<<std::endl; | ||
| os<<"authority: "<<this->hub[i]<<std::endl; | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you use essential's print utility instead of writing a specialized helper here? It is cluttering the file.
| // Functor for deviding each element with sum and taking the sqrt | ||
| template<typename T> | ||
| struct op{ | ||
| private: | ||
| float sum; | ||
| public: | ||
| op(float sum) : sum(sum){} | ||
| __device__ __host__ | ||
| T operator()(const T& x) const { | ||
| return sqrt(x/this->sum); | ||
| } | ||
| }; |
There was a problem hiding this comment.
You can use C++ lambda functions instead of functors.
| } | ||
| } | ||
|
|
||
| void update_iterator(){++this->iterator;} |
There was a problem hiding this comment.
You do not need this, iteration is automatically updated already, just use enactor's iteration for this instead.
| void update_auth(int dest_pos, int source_pos){ | ||
| gunrock::math::atomic::add(&this->auth_next_p[dest_pos], | ||
| hub_curr_p[source_pos]); | ||
| } | ||
|
|
||
| __device__ | ||
| void update_hub(int dest_pos, int source_pos){ | ||
| gunrock::math::atomic::add(&hub_next_p[dest_pos], | ||
| auth_curr_p[source_pos]); | ||
| } |
There was a problem hiding this comment.
One line functions? Either use __forceinline__ or just incorporate the contents of the function as the instruction you want to do. i.e. you do not need these to be separate functions. Just use the atomic add and comment that this updates hub or auth.
|
|
||
| bool is_converged(cuda::multi_context_t& context) override { | ||
| auto P = this->get_problem(); | ||
| return P->is_converged(); |
There was a problem hiding this comment.
Why isn't the contents of P->is_converged() just part of this function instead? Why does it have to call a function within a function to do something?
| } | ||
| void reset() override{} | ||
|
|
||
| bool is_converged(){ |
There was a problem hiding this comment.
This can just go in the enactor instead.
| auth_next_p = auth_next.data().get(); | ||
| hub_next_p = hub_next.data().get(); | ||
| } | ||
| void reset() override{} |
There was a problem hiding this comment.
The initial fills from https://github.com/gunrock/essentials/pull/83/files#diff-b62c127579eb994567fe8e548f95a87eba8fc88110834e01736092e7d816e04fR166-R172 can be part of reset instead of being part of the constructor. reset() should run on every run of the application to "reset" the intermediate data I am assuming?
| thrust::fill(policy, hub_next.begin(), hub_next.end(), 0); | ||
| } | ||
|
|
||
| void init() override{ |
There was a problem hiding this comment.
Init should be responsible for resizing the given vectors. (see other apps for examples on init and reset)
|
Merging this anyways. |
|
@neoblizz Sorry, I didn’t see this at that time :( |
Porting HITS algorithm for old GunRock