-
-
Notifications
You must be signed in to change notification settings - Fork 11.2k
[ROCm][Build][Bugfix] Fix ROCm base docker whls installation order #25415
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
[ROCm][Build][Bugfix] Fix ROCm base docker whls installation order #25415
Conversation
Signed-off-by: Gregory Shtrasberg <Gregory.Shtrasberg@amd.com>
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.
Code Review
This pull request refactors the ROCm base Dockerfile to improve the build process. Key changes include installing all wheel files in a single command to prevent dependency conflicts, and separating the FlashAttention build into its own stage to enable parallel builds. These are positive changes that enhance the robustness and efficiency of the Docker image creation. I have identified one high-severity issue related to the filtering of GPU architectures for the FlashAttention build, which could result in an incorrect build configuration. A suggestion to fix this is provided in the detailed comments.
…25415) Signed-off-by: Gregory Shtrasberg <Gregory.Shtrasberg@amd.com> Signed-off-by: yewentao256 <zhyanwentao@126.com>
…llm-project#25415) Signed-off-by: Gregory Shtrasberg <Gregory.Shtrasberg@amd.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
…llm-project#25415) Signed-off-by: Gregory Shtrasberg <Gregory.Shtrasberg@amd.com>
…llm-project#25415) Signed-off-by: Gregory Shtrasberg <Gregory.Shtrasberg@amd.com>
…llm-project#25415) Signed-off-by: Gregory Shtrasberg <Gregory.Shtrasberg@amd.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
…llm-project#25415) Signed-off-by: Gregory Shtrasberg <Gregory.Shtrasberg@amd.com>
Make all whls install at the same command to fix the missing dependency issue when triton_kernels would pull CUDA dependencies from pypi
Split FA into its own build step to run in parallel