don't build images w cuda 130 since we don't have flash attention wheels#3341
Conversation
📝 WalkthroughWalkthroughThe pull request modifies CI/build configuration and Docker setup. It comments out a CUDA 13.0 matrix entry in GitHub Actions workflows and changes the flash_attn installation condition in the Dockerfile from an exact PyTorch version match to a regex pattern matching any 2.9.x version. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/main.ymldocker/Dockerfile-base
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: PyTest from Source Dist (3.11, 2.9.0)
- GitHub Check: PyTest (3.11, 2.8.0)
- GitHub Check: PyTest (3.11, 2.9.0)
- GitHub Check: PyTest (3.11, 2.9.1)
- GitHub Check: PyTest from Source Dist (3.11, 2.8.0)
- GitHub Check: PyTest from Source Dist (3.11, 2.9.1)
- GitHub Check: build-base (130, 13.0.0, 3.11, 2.9.1, 9.0+PTX, Dockerfile-base)
- GitHub Check: build-base (128, 12.8.1, 3.11, 2.8.0, 7.0 7.5 8.0 8.6 8.7 8.9 9.0+PTX, Dockerfile-base)
- GitHub Check: build-base (128, 12.8.1, 3.11, 2.9.0, 7.0 7.5 8.0 8.6 8.7 8.9 9.0+PTX, Dockerfile-base)
- GitHub Check: build-base (128, 12.8.1, 3.11, 2.9.1, 7.0 7.5 8.0 8.6 8.7 8.9 9.0+PTX, Dockerfile-base)
- GitHub Check: build-base-uv (128, 12.8.1, 3.11, 2.8.0, 7.0 7.5 8.0 8.6 8.7 8.9 9.0+PTX, Dockerfile-uv-base)
- GitHub Check: build-base-uv (128, 12.8.1, 3.11, 2.9.0, 7.0 7.5 8.0 8.6 8.7 8.9 9.0+PTX, Dockerfile-uv-base)
- GitHub Check: build-base-uv (128, 12.8.1, 3.11, 2.9.1, 7.0 7.5 8.0 8.6 8.7 8.9 9.0+PTX, Dockerfile-uv-base)
- GitHub Check: build-base-uv (130, 13.0.0, 3.11, 2.9.1, 9.0+PTX, Dockerfile-uv-base)
🔇 Additional comments (2)
.github/workflows/main.yml (2)
34-38: LGTM!The CUDA 13.0 matrix entry is correctly disabled by commenting it out, which aligns with the PR objective to stop building images without flash attention wheel support.
101-105: LGTM!Consistent removal of CUDA 13.0 support across all build jobs.
| pip3 cache purge | ||
|
|
||
| RUN if [ "$PYTORCH_VERSION" = "2.9.1" ] && [ "$CUDA" = "128" ] ; then \ | ||
| RUN if [ "$PYTORCH_VERSION" =~ ^2\.9\.[0-9]+$ ] && [ "$CUDA" = "128" ] ; then \ |
There was a problem hiding this comment.
Critical syntax error: =~ requires double brackets in Bash.
The regex operator =~ only works with [[ (double brackets) in Bash, not with [ (single bracket/test command). This will cause a build failure or incorrect conditional evaluation.
🔎 Proposed fix
-RUN if [ "$PYTORCH_VERSION" =~ ^2\.9\.[0-9]+$ ] && [ "$CUDA" = "128" ] ; then \
+RUN if [[ "$PYTORCH_VERSION" =~ ^2\.9\.[0-9]+$ ]] && [[ "$CUDA" = "128" ]] ; then \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RUN if [ "$PYTORCH_VERSION" =~ ^2\.9\.[0-9]+$ ] && [ "$CUDA" = "128" ] ; then \ | |
| RUN if [[ "$PYTORCH_VERSION" =~ ^2\.9\.[0-9]+$ ]] && [[ "$CUDA" = "128" ]] ; then \ |
🤖 Prompt for AI Agents
In docker/Dockerfile-base around line 54, the conditional uses the Bash regex
operator with single brackets (`[ "$PYTORCH_VERSION" =~ ... ]`) which is
invalid; replace the single-bracket test with a Bash double-bracket conditional
(`[[ "$PYTORCH_VERSION" =~ ^2\.9\.[0-9]+$ ]] && [ "$CUDA" = "128" ]`) or convert
the entire check to a POSIX-safe alternative (e.g., use grep -E on the variable)
and ensure the Dockerfile uses bash (#!/usr/bin/env bash or `SHELL
["/bin/bash","-c"]`) if relying on `[[ ... =~ ... ]]`.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Description
Motivation and Context
How has this been tested?
Screenshots (if appropriate)
Types of changes
Social Handles (Optional)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.