-
Notifications
You must be signed in to change notification settings - Fork 47
Move towards using MPL in the GPU version #335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
|
This is great step!
So the logic needs to be a bit different for this to work. |
There are preexisting references to |
|
Following offline discussions with @wdeconinck, I've added support for the MPI_F08 feature (on by default) of FIAT. This further reduces the configurations where it's necessary to call MPI directly (what I call "raw" MPI). The only remaining configuration in fact is when ecTrans is being built against a FIAT version earlier than {next version to be released} (a new release with MPI_F08 compatibility hasn't been made yet). If we in future made {next FIAT version to be released} as the minimum supported FIAT version, we could simply delete all raw MPI calls. I will do some testing to make sure everything is working, before this can be merged. |
|
Problems on LUMI... I wonder if we have to add an exception for CCE. |
I think this is again this Cray issue biting us: #157 (comment) |
|
Unfortunately I think we will have to enable |
This is set when we enabled GPU-aware communication and FIAT doesn't support MPI_F08 (either because it's disabled, or because we're using an older version of FIAT which doesn't have any MPI_F08 at all).
8f77c9d to
d9dba97
Compare
|
Wow, what a nightmare. After a lot of tedious debugging, I noticed that I had removed the |
|
During debugging I noticed some issues with |
|
The plot thickens: |
Co-authored-by: Willem Deconinck <[email protected]>
| #ifdef PARKINDTRANS_SINGLE | ||
| #define TRMTOL_DTYPE MPI_REAL4 | ||
| #define TRMTOLAD_DTYPE MPI_REAL4 | ||
| #else | ||
| #define TRMTOL_DTYPE MPI_REAL8 | ||
| #define TRMTOLAD_DTYPE MPI_REAL8 | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section still needs to be guarded by #ifdef USE_RAW_MPI because MPI_REAL4 and MPI_REAL8 are normally not available. I wonder why this worked.
Do we have similar construct in the other tr*to* routines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TRMTOLAD_DTYPE is only referenced inside of #ifdef USE_RAW_MPI regions, that's why there's no compile error.
This is a slightly less brute-force alternative to PR #334 which also lays the groundwork for eventually relying entirely on MPL in the GPU code path. Let me explain...
With this branch, if you disable
GPU_AWARE_MPI, an MPI library is not required by ecTrans. No such library will be linked against and there will be no calls to MPI in any compiled object code. Whether MPI is called "under the hood" of MPL depends entirely on whether you compiled FIAT with or without MPI. In the latter case, the MPI serial fallback will be used. This means you can test on GPU platforms without an MPI installation by simply building FIAT without MPI and disablingGPU_AWARE_MPI.For now,
GPU_AWARE_MPIrequires direct calls to MPI, hence only for that configuration do we need to link against MPI::MPI_Fortran explicitly. Eventually we should have support to pass GPU buffers to MPL, and when that happens we can finally delete all references to MPI from ecTrans and rely entirely on MPL, much as we already do for the CPU version.