Feature/hierarchical collectives #6668
Conversation
|
Can one of the admins verify this patch? |
| if (generation == 0) | ||
| { | ||
| return hpx::make_exceptional_future<T>(HPX_GET_EXCEPTION( | ||
| hpx::error::bad_parameter, "hpx::collectives::scatter_to", |
There was a problem hiding this comment.
The function name needs to be corrected
| for (int i = 0; i < communicators.size()-1;i++) | ||
| { | ||
| current_communicator = std::get<0>(communicators[i]); | ||
| current_local_result = broadcast_to(current_communicator, std::move(current_local_result), generation, this_site_arg(0)).get(); |
There was a problem hiding this comment.
We do have a synchronous overload of broadcast. Please consider using it instead.
| current_communicator = std::get<0>(communicators[i]); | ||
| current_local_result = broadcast_to(current_communicator, std::move(current_local_result), generation, this_site_arg(0)).get(); | ||
| } | ||
| current_communicator = std::get<0>(communicators[communicators.size()-1]); |
There was a problem hiding this comment.
| current_communicator = std::get<0>(communicators[communicators.size()-1]); | |
| current_communicator = std::get<0>(communicators.back()); |
| for (int i = 0; i < communicators.size()-1;i++) | ||
| { | ||
| current_communicator = std::get<0>(communicators[i]); | ||
| current_local_result = broadcast_to(current_communicator, std::move(current_local_result), generation, this_site_arg(0)).get(); |
There was a problem hiding this comment.
Please use HPX_MOVE instead of std::move for HPX implementation files (not tests or examples, though).
| communicator current_communicator = std::get<0>(communicators[0]); | ||
| int current_site = std::get<1>(communicators[0]); |
There was a problem hiding this comment.
| communicator current_communicator = std::get<0>(communicators[0]); | |
| int current_site = std::get<1>(communicators[0]); | |
| auto [current_communicator, current_site] = communicators[0]; |
| for (auto& row : non_flat_vector) { | ||
| current_local_result.insert(current_local_result.end(), std::make_move_iterator(row.begin()), std::make_move_iterator(row.end())); | ||
| } | ||
| return std::move(current_local_result); |
There was a problem hiding this comment.
Never use std::move on a local variable being returned from a function. This disables RVO in the compiler.
| return hpx::make_exceptional_future<T>(HPX_GET_EXCEPTION( | ||
| hpx::error::bad_parameter, "hpx::collectives::scatter_to", | ||
| "the generation number shouldn't be zero")); | ||
| } */ |
There was a problem hiding this comment.
Please remove code that has been commented out.
| for (int i = communicators.size()-1; i > 0;i--) | ||
| { | ||
| current_communicator = std::get<0>(communicators[i]); | ||
| std::vector<std::vector<T>> in_between_result = gather_here(current_communicator, std::move(current_local_result), generation, this_site_arg(0)).get(); |
There was a problem hiding this comment.
We do have a synchronous version of gather. Please consider using it instead.
| for (int i = communicators.size()-1; i > 0;i--) | ||
| { | ||
| current_communicator = std::get<0>(communicators[i]); | ||
| std::vector<std::vector<T>> in_between_result = gather_here(current_communicator, std::move(current_local_result), generation, this_site_arg(0)).get(); |
There was a problem hiding this comment.
Moving from current_local_result repeatedly is not what you want.
| if (generation == 0) | ||
| { | ||
| return hpx::make_exceptional_future<T>(HPX_GET_EXCEPTION( | ||
| hpx::error::bad_parameter, "hpx::collectives::scatter_to", |
There was a problem hiding this comment.
The function name needs to be corrected.
3 participants
This is the hierarchical collectives and hierarchical Communicator code used in the Bachelorthesis 'Hierarchical Collectives for HPX'
This is merely a proof of concept.