Fixes to acoll component #13575
Fixes to acoll component #13575
Conversation
Adds some bug fixes to the acoll component. - gather is corrected to work for non-zero root ranks when three stage algorithm is used. - Busy waits in shared memory bcast and barrier are changed to avoid random hangs. - Overrides with command line arguments are properly taken care of in bcast algorithm selection. - Multinode path of reduce and allreduce is fixed to use the hierarchical algorithms of acoll. - Compile time warnings are removed. Signed-off-by: Nithya V S <[email protected]>
edgargabriel
left a comment
edgargabriel
left a comment
There was a problem hiding this comment.
a few minor formatting things, but otherwise it looks good in my opinion.
| } | ||
|
|
||
| if (MPI_SUCCESS != err) { | ||
| if (NULL != inplacebuf_free) { free(inplacebuf_free); } |
There was a problem hiding this comment.
please move the free(inplace_buf) into a separate line (two more instances of the same issue below).
| } | ||
| } | ||
|
|
||
| if (num_nodes == 1) { |
There was a problem hiding this comment.
the PR has a number of places where the constant is not listed at the first part of the comparison. Its most relevant for the == options, but does also apply for != and other operands (at many places you have the correct order, e.g. a few lines above at the if (MPI_SUCCESS != err)). Please double check the PR for that.
There was a problem hiding this comment.
Done here and in other places in multiple files.
Changes in formatting to adhere to ompi coding style. Signed-off-by: Nithya V S <[email protected]>
Fixes a condition for freeing temporary buffer in acoll reduce. Signed-off-by: Nithya V S <[email protected]>
amd-nithyavs
left a comment
amd-nithyavs
left a comment
There was a problem hiding this comment.
Made the changes along with one more fix related to a buffer free.
…ixes Fixes to acoll component (cherry picked from commit e47e9dd)
5 participants
Adds some bug fixes to the acoll component.