Support building with GASNet-EX and MPI backends#322
Support building with GASNet-EX and MPI backends#322manopapad wants to merge 4 commits intonv-legate:branch-22.10from
Conversation
| dest="gasnet", | ||
| action="store_true", | ||
| "--network", | ||
| dest="networks", |
There was a problem hiding this comment.
We can feed multiple networks to Realm, but I am not advertising this currently, since this feature is not fully working on the Realm side.
|
One thing to note here is that previously we were assuming that "using networking" implied "using gasnet", and because gasnet requires MPI at build time, and initializes it at runtime, we could assume that "using networking" automatically meant "using MPI". This will not necessarily be the case for all backends, so we could presumably have two separate settings, USE_NETWORK and USE_MPI. I chose to continue having a single USE_NETWORK setting; picking any networking backend, whether it requires MPI itself or not, also enables MPI support. Here is what USE_NETWORK=1 now implies regarding MPI:
|
| log_coll.fatal( | ||
| "MPI has not been initialized, it should be initialized by " | ||
| "GASNet"); | ||
| "the networking backend"); |
There was a problem hiding this comment.
@eddy16112 In the future we will want to also expect the case where the networking backend doesn't initialize MPI, and in that case do it ourselves.
There was a problem hiding this comment.
Can we have one variable GASNET_CONDUIT to tell which conduit is enabled for gasnet and another one LEGATE_NETWORK to tell which network backend is used? Then we can add a more comprehensive check here to check if we need to init MPI or not.
There was a problem hiding this comment.
Would it be simpler to just query MPI directly, using the MPI_Initialized call?
There was a problem hiding this comment.
Yes, we do use MPI_Initialized. What I mean is we can have an extra level of verification. For example: if MPI_Initialized == true, then GASNET_CONDUIT should be mpi or ibv, otherwise, something is wrong.
There was a problem hiding this comment.
You can check these macros, defined in realm_defines.h:
// one of these is defined, depending on the network used:
#define REALM_USE_GASNET1
/* #undef REALM_USE_GASNETEX */
/* #undef REALM_USE_MPI */
// if using gasnet, one of these will be defined:
#define GASNET_CONDUIT_MPI 1
/* #undef GASNET_CONDUIT_IBV */
/* #undef GASNET_CONDUIT_UDP */
/* #undef GASNET_CONDUIT_ARIES */
/* #undef GASNET_CONDUIT_GEMINI */
/* #undef GASNET_CONDUIT_PSM */
/* #undef GASNET_CONDUIT_UCX */
/* #undef GASNET_CONDUIT_OFI */
| "--gasnet", | ||
| dest="gasnet", | ||
| action="store_true", | ||
| "--network", |
There was a problem hiding this comment.
CC @trxcllnt @jefflarkin: Note this modification to an install.py flag, that might conflict with your changes.
|
@manopapad looks good to me. One question though: is there a particular reason that we allow multiple networks? When both gasnet1 and gasnet-ex are given, the latter is always chosen by the build. And the MPI backend is activated only when |
@streichler had suggested we are at least able to pass multiple networks to Realm, in preparation for when Realm can actually support multiple network modules under the same build. |
|
Realm has partially-implemented support for being able to run with multiple networks active. Although there are no immediate plans to complete that implementation, there is a desire to avoid structural limitations in build flows and the like that assume at most one network module is permitted. |
that's what I guessed. I suggest we at least raise a warning when someone tries to do it, as it doesn't have the intended effect at the moment. |
|
I added the warning as requested, but I was asked to sit on this PR while the cmake changes are finalized, and will rebase over that when it lands. |
|
Deprecated by #384 |
* Better heuristic to locate the gtest binary * Make the fail-to-find-build-dir a soft failure
No description provided.