Feature: Add basic NCCL communication space backend #128
Conversation
CMake module for finding NCCL was taken from pytorch repo. Need to look into licensing rules.
This change allows users to avoid having to use backend-specific reduction types when calling KokkosComm functions.
NCCL can now be enabled without explicitly requiring enabling MPI. However, need to check that MPI is still an available dependency as it is the fallback communication backend!
| #if defined(KOKKOSCOMM_ENABLE_MPI) | ||
| class Mpi; | ||
| using DefaultCommunicationSpace = Mpi; | ||
| using FallbackCommunicationSpace = Mpi; | ||
| #elif defined(KOKKOSCOMM_ENABLE_NCCL) | ||
| class Mpi; |
There was a problem hiding this comment.
I think I should depend on MPI in this case.
| return true; | ||
| } | ||
| } | ||
| return false; |
There was a problem hiding this comment.
Should you block one request if none of the input is ready?
src/KokkosComm/collective.hpp
Outdated
| KokkosExecutionSpace ExecSpace = Kokkos::DefaultExecutionSpace, | ||
| CommunicationSpace CommSpace = DefaultCommunicationSpace> | ||
| auto reduce(Handle<ExecSpace, CommSpace>& h, const SendView& sv, RecvView& rv, int root) -> Req<CommSpace> { | ||
| return Impl::Reduce<SendView, RecvView, RedOp, ExecSpace, CommSpace>(h, sv, rv, root); |
There was a problem hiding this comment.
I do not exactly understand how it will work. Don't we need to call it execute()?
There was a problem hiding this comment.
You are right, I fixed this in 9c185af (#128).
| space.cuda_stream()); | ||
| RecvPacker::unpack_into(space, rv, recv_args.view); | ||
| } else { | ||
| space.fence(); // is this fence necessary? |
There was a problem hiding this comment.
I would remove this fence.
| space.cuda_stream()); | ||
| RecvPacker::unpack_into(space, rv, recv_args.view); | ||
| } else { | ||
| space.fence(); // is this fence necessary? |
There was a problem hiding this comment.
Not needed in my opinion.
| namespace KokkosComm::Experimental::nccl::Impl::Packer { | ||
|
|
||
| template <KokkosView View> | ||
| struct NcclArgs { |
There was a problem hiding this comment.
I think PackedViewNccl or something like that will be a better name. Looking at the following code, I thought NCClArgs was an NCCL thing.
As per @cedricchevalier review comment.
This changes matches the high-level API for point-to-point communications.
Signed-off-by: Carl Pearson <cwpears@sandia.gov>
Signed-off-by: Carl Pearson <cwpears@sandia.gov>
Signed-off-by: Carl Pearson <cwpears@sandia.gov>
Signed-off-by: Carl Pearson <cwpears@sandia.gov>
Signed-off-by: Carl Pearson <cwpears@sandia.gov>
Signed-off-by: Carl Pearson <cwpears@sandia.gov>
|
I think it builds now. My workstation has GPU-aware MPI problems, so I need to fix those before continuing. |
This is still very much WIP and cannot build at the moment. I am simply opening the PR so that folks meeting at SC24 can exchange on it. 👍