-
Notifications
You must be signed in to change notification settings - Fork 520
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
pt: fix compile error when use intel mpi #3919
Conversation
WalkthroughIn the latest update, checks for CUDA support within MPI were integrated into the build system and source code. Conditional logic was added to manage CUDA awareness based on whether Changes
Sequence Diagram(s)sequenceDiagram
participant CMakeLists.txt
participant Compilation
participant comm.cc
participant MPI
participant Execution
CMakeLists.txt->>+Compilation: Check for MPIX_Query_cuda_support
Compilation-->>CMakeLists.txt: Define NO_CUDA_AWARE if absent
comm.cc-->>comm.cc: Conditional compilation (NO_CUDA_AWARE)
MPI-->>comm.cc: Provide MPI version and CUDA support
Execution->>comm.cc: Instantiate Border class
comm.cc-->>Execution: Adjust behavior based on CUDA awareness
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
source/op/pt/comm.cc (1)
Line range hint
204-345
: Consistency check and optimization suggestion forbackward
method.The implementation is consistent with the
forward
method and correctly handles MPI and CUDA logic. Consider optimizing memory usage and data transfers, especially when CUDA is not aware, to improve performance.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #3919 +/- ##
==========================================
- Coverage 82.88% 82.87% -0.01%
==========================================
Files 520 520
Lines 50678 50678
Branches 3018 3011 -7
==========================================
- Hits 42003 42000 -3
- Misses 7738 7740 +2
- Partials 937 938 +1 ☔ View full report in Codecov by Sentry. |
I can find grep MPIX_Query_cuda_support ~/intel/oneapi/mpi/2021.13/include/mpi.h
int MPIX_Query_cuda_support(void) MPICH_API_PUBLIC;
int PMPIX_Query_cuda_support(void) MPICH_API_PUBLIC; |
My intel MPI version is 2021.5.1 and there is no definition for |
Perhaps use CMake's |
for more information, see https://pre-commit.ci
`MPIX_Query_cuda_support()` is not defined in intelMPI <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced conditional handling for CUDA awareness based on the MPI version to improve compatibility and performance for CUDA-enabled environments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
MPIX_Query_cuda_support()
is not defined in intelMPISummary by CodeRabbit