OpenMPI + CUDA Compatiblity#2279
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2279 +/- ##
==========================================
- Coverage 91.74% 91.73% -0.02%
==========================================
Files 87 87
Lines 14120 14148 +28
==========================================
+ Hits 12955 12979 +24
- Misses 1165 1169 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
brownbaerchen
left a comment
There was a problem hiding this comment.
I have a bunch of small comments about clarifying stuff. I don't know how MPI datatypes work and therefore don't understand some of the changes.
Most changes look really good, though!
There was a problem hiding this comment.
Can we rename this variable to something more expressive? Maybe cuda_is_available?
Co-authored-by: Thomas Saupe <39156931+brownbaerchen@users.noreply.github.com>
Co-authored-by: Thomas Saupe <39156931+brownbaerchen@users.noreply.github.com>
Co-authored-by: Thomas Saupe <39156931+brownbaerchen@users.noreply.github.com>
Benchmarks results - Sponsored by perunFTPx86-AMD-Milan-Mi250 - CPU - N4 - RUNTIME
FTPx86-AMD-Milan-Mi250 - GPU - N4 - RUNTIME
Grafana Dashboard |
|
I've addressed the comments, and for now disabled rocm buffer compatibility with MPI. Should be addressed later in a different issue/pr. Should be ready to merge if after a review and if all tests pass. |
brownbaerchen
left a comment
There was a problem hiding this comment.
I still think the incompatibilities determination is difficult to maintain because it's a bit confusing.. I would be open to merging this anyways if you don't have bandwidth to fix this, though. GPU-aware MPI compatibility will be very nice!
| else None | ||
| ) | ||
| gpu_comp = (rocm and CUDA_IS_ACTUALLY_ROCM) or (cuda and not CUDA_IS_ACTUALLY_ROCM) | ||
| gpu_comp = gpu_comp and isinstance(device_incompatibilities, list) |
There was a problem hiding this comment.
Hm? This deactivates GPU computation by default, yes? If no incompatibilities have been recorded, the default is device_incompatibilities=None, right? Maybe that's what we want, though..
There was a problem hiding this comment.
GPU compatibility is set to true if the library and environment say indicate that it is compatible (from ompi_info flags, for example), and if the list of incompatiblities is a list. If it is False or any other object, it will disable gpu support.
Introduces an incompatibility list for different communication backends, in order to safely enable GPU compatible MPI communication.
Changes
_moveToCompDeviceDoes this affect performance
resplit_(None)) have shown massive slowdowns when running the tests. We might want to add certain functions to the incompatibility list for performance reasons.