[DR] Hierarchical Communicator Design

ammarhakim commented 4 months ago

Prototype/Production Branches

Please see the branch multib-app for work on communicators. This is the same branch in which the multi-block work is happening as these two issues are connected. I consider this to be a production branch

Introduction

Communicators need to be arrange hierarchically to reflect the hierarchy in the App system:

Some functionality is purely communicator based, for example, ranks, size of communicator, barrier, etc
Some functionality requires a decomposition. For example sync of ghost-cells in each block, all-gather etc
Some functionality requires both a decomposition and a topology, for example, inter-block communication

Hence, a refactoring of the existing comm system is needed to support all these various scenarios.

Principles

Each communicator type (null, MPI and nccl) must support the same functionality. This is critical to avoid any if-statements except in communicator creation functions
Connected with the above principle is that there should not be "host" methods in nccl communicators. This breaks layering by indicating that we are dealing with a CUDA communicator. The specific comm type should not be visible or matter to any user code (except for creation)
No leakage of internal data should occur, for example, MPI specific pointers should not be returned from methods (for obvious reasons)

Proposed Design

I propose that we add a new set of methods that work on inter-block communications. The design needs some thought: should this be a new set of objects built on top of the existing comm objects, or just added to the present comm object. The latter has the advantage that we already have communicators used extensively everywhere in the code and so no need things will be needed. The disadvantage is that the IFCs become pretty fat.

We should aim for hierarchical methods. For example, a all-reduce on a communicator that is topology aware should call the block-level all-reduce and then do what is needed to all-reduce based on the topology. This frees up the user from worrying about how to do these operations himself.

One code implementation option is to introduce more structure into the gkyl_comm structure: perhaps we put the function pointers in nested structs for cleaner code, though this is only a cosmetic thing (perhaps not).

ammarhakim commented 3 months ago

I have performed some major surgery to the comm interfaces. This cleans up the code significantly, removing all internal details to private headers not accessible to a user. This is achieved by an additional level of indirection. The gkyl_comm object now simply looks like:

struct gkyl_comm {
  char id[128]; // string ID for communcator
  bool has_decomp; // flag to indicate if comm has an associated decomp
  const struct gkyl_rect_decomp *decomp; // decomp, if it exists, or NULL

  struct gkyl_ref_count ref_count; // reference count
};

No function pointers are in this struct. Instead a new gkyl_comm_priv struct holds them. As the name indicates this struct is private and not visible to the user. This greatly cleans up the main gkyl_comm.h header as all nasty function typedefs are not leaked to the user now. (We need to do this across the code, I believe).

ammarhakim commented 1 week ago

This DR is completed and can be closed. Some more work remains in inter-block comm but the work that was proposed here is completed.

ammarhakim / gkylzero