Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Docker build assets to create a CUDA-enabled (nvidia/cuda:13.1.1-cudnn-devel-ubuntu24.04) Python 3.12 environment: a Dockerfile, scripts to install Miniforge/Conda and CUDA-aware Python packages, plus a requirements.txt listing project Python dependencies for the container. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces comprehensive Docker support for the evaluation process. By providing a dedicated Dockerfile and associated installation scripts, it aims to standardize the development and evaluation environment, ensuring reproducibility and simplifying setup for all contributors. This change streamlines dependency management and provides a robust, isolated environment for running MLsys26 evaluations. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds Docker support for an evaluation environment. The changes are generally good, introducing a Dockerfile and associated scripts to build a container with a specific Python and CUDA environment. I've identified a critical issue with an incorrect CUDA version for PyTorch that would cause the build to fail. I've also provided several suggestions to improve the robustness, reproducibility, and maintainability of the Docker build process, such as pinning dependency versions, cleaning up intermediate files, and consolidating package installation steps.
| # Install torch and other python packages | ||
| COPY docker/requirements.txt /install/requirements.txt | ||
| COPY docker/install/install_python_packages.sh /install/install_python_packages.sh | ||
| RUN bash /install/install_python_packages.sh cu130 |
There was a problem hiding this comment.
The base image nvidia/cuda:13.1.1-cudnn-devel-ubuntu24.04 uses CUDA 13.1. The correct PyTorch wheel for CUDA 13.1 is cu131, not cu130. Using cu130 will cause the build to fail as the corresponding torch wheel does not exist.
RUN bash /install/install_python_packages.sh cu131
| # Set home directory | ||
| WORKDIR /workspace | ||
|
|
||
| RUN echo "source activate py312" >> ~/.bashrc |
There was a problem hiding this comment.
The command RUN echo "source activate py312" >> ~/.bashrc is not a robust way to manage the conda environment. It only affects interactive shells for the root user and has no effect on subsequent RUN commands in the Dockerfile. The PATH is already correctly configured via the ENV instructions on lines 23-24, which is the correct approach for build-time commands. For interactive use, users can be instructed to run conda activate py312 manually. This line adds little value and can be confusing.
| wget -O Miniforge3.sh "https://github.com/conda-forge/miniforge/releases/latest/download/Miniforge3-$(uname)-$(uname -m).sh" | ||
| bash Miniforge3.sh -b -p $1 |
There was a problem hiding this comment.
There are two issues with the current implementation:
- Reproducibility: Downloading from a
.../latest/...URL makes the Docker build non-reproducible. A new release of Miniforge could introduce breaking changes. It's better to pin to a specific version (e.g.,24.3.0-0) for consistent builds. - Cleanup: The downloaded installer script
Miniforge3.shis not removed after execution. This adds an unnecessary file to the Docker image layer, increasing its size.
| wget -O Miniforge3.sh "https://github.com/conda-forge/miniforge/releases/latest/download/Miniforge3-$(uname)-$(uname -m).sh" | |
| bash Miniforge3.sh -b -p $1 | |
| wget -O Miniforge3.sh "https://github.com/conda-forge/miniforge/releases/download/24.3.0-0/Miniforge3-$(uname)-$(uname -m).sh" | |
| bash Miniforge3.sh -b -p $1 | |
| rm Miniforge3.sh |
| set -e | ||
| set -u |
There was a problem hiding this comment.
For robustness, it's good practice to include set -o pipefail in bash scripts to ensure that a pipeline command is considered to have failed if any of its components fail. You can also combine the set options into a single line for conciseness.
| set -e | |
| set -u | |
| set -euo pipefail |
| pip3 install -r /install/requirements.txt | ||
| pip3 install responses pytest scipy build cuda-python | ||
|
|
||
| # Install cudnn package based on CUDA version | ||
| if [[ "$CUDA_VERSION" == *"cu13"* ]]; then | ||
| pip3 install --upgrade cuda-python==13.0 | ||
| pip3 install "nvidia-cudnn-cu13>=9.14.0.64" | ||
| else | ||
| pip3 install --upgrade cuda-python==12.* | ||
| pip3 install "nvidia-cudnn-cu12>=9.14.0.64" | ||
| fi | ||
|
|
||
| # Contest-specific packages | ||
| pip3 install tilelang cuda-tile cupti-python pandas |
There was a problem hiding this comment.
The current package installation can be improved by consolidating pip commands and removing a redundant installation. Specifically:
- The multiple
pip3 installcalls for individual packages can be combined into one. cuda-pythonis installed on line 13 and then again in the conditional block, which is redundant.
This refactoring improves readability and allows pip to resolve dependencies more efficiently.
| pip3 install -r /install/requirements.txt | |
| pip3 install responses pytest scipy build cuda-python | |
| # Install cudnn package based on CUDA version | |
| if [[ "$CUDA_VERSION" == *"cu13"* ]]; then | |
| pip3 install --upgrade cuda-python==13.0 | |
| pip3 install "nvidia-cudnn-cu13>=9.14.0.64" | |
| else | |
| pip3 install --upgrade cuda-python==12.* | |
| pip3 install "nvidia-cudnn-cu12>=9.14.0.64" | |
| fi | |
| # Contest-specific packages | |
| pip3 install tilelang cuda-tile cupti-python pandas | |
| pip3 install -r /install/requirements.txt | |
| pip3 install \ | |
| responses \ | |
| pytest \ | |
| scipy \ | |
| build \ | |
| tilelang \ | |
| cuda-tile \ | |
| cupti-python \ | |
| pandas | |
| # Install cudnn package based on CUDA version | |
| if [[ "$CUDA_VERSION" == *"cu13"* ]]; then | |
| pip3 install --upgrade cuda-python==13.0 | |
| pip3 install "nvidia-cudnn-cu13>=9.14.0.64" | |
| else | |
| pip3 install --upgrade cuda-python==12.* | |
| pip3 install "nvidia-cudnn-cu12>=9.14.0.64" | |
| fi |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
docker/Dockerfile.mlsys26 (1)
6-13: Use--no-install-recommendsin the APT layer.This keeps the base image smaller and reduces unnecessary packages in the evaluation container.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/Dockerfile.mlsys26` around lines 6 - 13, Modify the RUN apt-get layer so apt-get install uses the --no-install-recommends flag to avoid pulling unnecessary recommended packages; specifically update the RUN instruction that currently starts with "apt-get update && apt-get install -y \ curl \ git \ ..." to "apt-get update && apt-get install -y --no-install-recommends \ ..." (keeping the same package list and the trailing "&& rm -rf /var/lib/apt/lists/*").docker/requirements.txt (1)
1-13: Freeze the evaluation dependency set.Most entries here are floating, so identical Docker builds can resolve different Python/CUDA stacks over time. For an evaluation image, please install from a fully pinned requirements/constraints file instead of lower bounds plus bare package names.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/requirements.txt` around lines 1 - 13, The requirements file currently lists many floating dependencies (e.g., apache-tvm-ffi, torch, numpy, nvidia-cudnn-frontend, nvidia-cutlass-dsl) which can resolve to different wheels over time; replace this with a fully pinned/locked dependency set by generating a freeze/constraints file (e.g., requirements-pinned.txt or constraints.txt) that records exact package==version and hashes, update docker/requirements.txt to reference that pinned file (or use pip with --require-hashes/--constraint) and ensure the Docker build installs from the pinned file instead of bare package names or lower bounds so builds are reproducible.docker/install/install_python_packages.sh (1)
11-25: Pin the CUDA wheel set and installcuda-pythononly once.Line 13 installs
cuda-pythonunqualified, then Lines 16-22 immediately replace it with a versioned install. That adds unnecessary resolver churn, and the floatingtorch/nvidia-cudnn-*installs make this image drift across rebuilds. Please drop the unconditionalcuda-pythoninstall and pin the CUDA-dependent wheels explicitly.Proposed cleanup
-pip3 install responses pytest scipy build cuda-python +pip3 install responses pytest scipy build if [[ "$CUDA_VERSION" == *"cu13"* ]]; then - pip3 install --upgrade cuda-python==13.0 + pip3 install --upgrade "cuda-python==13.0.*" pip3 install "nvidia-cudnn-cu13>=9.14.0.64" else pip3 install --upgrade cuda-python==12.* pip3 install "nvidia-cudnn-cu12>=9.14.0.64" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/install/install_python_packages.sh` around lines 11 - 25, Remove the unconditional unpinned install of cuda-python and instead pin CUDA-related wheels only once based on the CUDA_VERSION check: drop the standalone "pip3 install cuda-python" invocation and move to using the versioned installs inside the if/else that reference CUDA_VERSION (e.g., install cuda-python==13.0 when CUDA_VERSION contains "cu13" and cuda-python==12.* otherwise), and pin the torch and nvidia-cudnn packages (torch install via index-url and "nvidia-cudnn-cu13"/"nvidia-cudnn-cu12") to fixed versions to avoid resolver churn and image drift; keep the other requirements installs (pip3 install -r /install/requirements.txt and contest packages like tilelang, cuda-tile, cupti-python, pandas) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/Dockerfile.mlsys26`:
- Around line 37-41: The Dockerfile prepares /opt/pip-user and sets
PYTHONUSERBASE/PATH but never drops root; create an unprivileged user (e.g.,
addgroup/adduser or use groupadd/useradd to make 'appuser' with a fixed
uid/gid), chown the /opt/pip-user directory and any other runtime dirs to that
user, and add a USER appuser instruction after the build/setup steps so the
container runs non-root; reference the existing RUN mkdir -p /opt/pip-user and
ENV PYTHONUSERBASE=/opt/pip-user / ENV PATH="/opt/pip-user/bin:$PATH" when
locating where to chown and switch users.
- Around line 26-27: The LD_LIBRARY_PATH entry set by the ENV LD_LIBRARY_PATH
line points to a non-existent pip wheel directory
(site-packages/nvidia/cu13/lib/) so pip-installed nvidia-cudnn-cu13 libs are not
taking precedence; update the path referenced by the ENV LD_LIBRARY_PATH to the
actual wheel install location site-packages/nvidia/cudnn/lib/ so pip-installed
libraries (from the installer script that installs nvidia-cudnn-cu13) are found
before base image libraries and verify the ENV name LD_LIBRARY_PATH remains
unchanged.
In `@docker/install/install_python.sh`:
- Around line 9-10: Replace the moving "latest" download and unconditional
execution of Miniforge in the install script: pin a concrete Miniforge release
by using its exact release filename/URL instead of the `latest` path, download
that file to a stable name (e.g., Miniforge3.sh), compute and verify its SHA256
checksum (compare output of sha256sum or shasum -a 256 against the expected
checksum string) and abort the script if the checksum does not match, then only
run `bash Miniforge3.sh -b -p $1` after verification; also remove the installer
after successful installation to avoid leaving executables around.
- Line 12: The conda create invocation "$1/bin/conda create -n $2 python=3.12"
will hang in non-interactive Docker builds; update the command used (the
invocation of conda in the installer script) to run non-interactively by adding
the -y flag and quote the variables for robustness (e.g., quote $1 and $2 and
the package spec) so the command becomes non-interactive and safe with spaces in
paths or env names.
---
Nitpick comments:
In `@docker/Dockerfile.mlsys26`:
- Around line 6-13: Modify the RUN apt-get layer so apt-get install uses the
--no-install-recommends flag to avoid pulling unnecessary recommended packages;
specifically update the RUN instruction that currently starts with "apt-get
update && apt-get install -y \ curl \ git \ ..." to "apt-get update && apt-get
install -y --no-install-recommends \ ..." (keeping the same package list and the
trailing "&& rm -rf /var/lib/apt/lists/*").
In `@docker/install/install_python_packages.sh`:
- Around line 11-25: Remove the unconditional unpinned install of cuda-python
and instead pin CUDA-related wheels only once based on the CUDA_VERSION check:
drop the standalone "pip3 install cuda-python" invocation and move to using the
versioned installs inside the if/else that reference CUDA_VERSION (e.g., install
cuda-python==13.0 when CUDA_VERSION contains "cu13" and cuda-python==12.*
otherwise), and pin the torch and nvidia-cudnn packages (torch install via
index-url and "nvidia-cudnn-cu13"/"nvidia-cudnn-cu12") to fixed versions to
avoid resolver churn and image drift; keep the other requirements installs (pip3
install -r /install/requirements.txt and contest packages like tilelang,
cuda-tile, cupti-python, pandas) unchanged.
In `@docker/requirements.txt`:
- Around line 1-13: The requirements file currently lists many floating
dependencies (e.g., apache-tvm-ffi, torch, numpy, nvidia-cudnn-frontend,
nvidia-cutlass-dsl) which can resolve to different wheels over time; replace
this with a fully pinned/locked dependency set by generating a
freeze/constraints file (e.g., requirements-pinned.txt or constraints.txt) that
records exact package==version and hashes, update docker/requirements.txt to
reference that pinned file (or use pip with --require-hashes/--constraint) and
ensure the Docker build installs from the pinned file instead of bare package
names or lower bounds so builds are reproducible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ae342f9-de34-4e81-b51c-5d9f6dd27b04
📒 Files selected for processing (4)
docker/Dockerfile.mlsys26docker/install/install_python.shdocker/install/install_python_packages.shdocker/requirements.txt
| # Ensure pip-installed nvidia libs take precedence | ||
| ENV LD_LIBRARY_PATH="/opt/conda/envs/py312/lib/python3.12/site-packages/nvidia/cu13/lib/:$LD_LIBRARY_PATH" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the Dockerfile
cat -n docker/Dockerfile.mlsys26 | head -35Repository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 1378
🏁 Script executed:
# Check the install script
cat -n docker/install/install_python_packages.sh | head -30Repository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 1115
🏁 Script executed:
# Search for any other references to LD_LIBRARY_PATH in the codebase
rg "LD_LIBRARY_PATH" --type dockerfile --type bashRepository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 120
🏁 Script executed:
# Search for LD_LIBRARY_PATH references in all file types
rg "LD_LIBRARY_PATH"Repository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 218
🏁 Script executed:
# Check if there's any documentation or comments about NVIDIA wheel structure
rg "nvidia/cudnn" -A 2 -B 2Repository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 68
🏁 Script executed:
# Check requirements.txt to understand dependencies
cat docker/requirements.txtRepository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 273
🏁 Script executed:
# Search for any documentation or comments about NVIDIA package structure
rg -i "cudnn" -i "library"Repository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 133
🏁 Script executed:
# Check if there are any other Dockerfiles or setup files that might reference nvidia wheels
find . -name "Dockerfile*" -o -name "*.md" | head -20Repository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 125
🏁 Script executed:
# Look at README or documentation files for context
find . -name "README*" -o -name "*.md" | xargs cat 2>/dev/null | head -100Repository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 3390
🏁 Script executed:
# Search for any test files or validation that might use these library paths
rg "site-packages" -iRepository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 218
🏁 Script executed:
# Look for any other references to nvidia packages or wheel structures
rg "nvidia-" --type bash --type pythonRepository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 114
🏁 Script executed:
# Check if there are any comments or documentation about CUDA/cuDNN versions
rg "cu13\|cudnn" -i -B 2 -A 2Repository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 68
🏁 Script executed:
# Search for any documentation about NVIDIA packages in the codebase
find . -name "*.txt" -o -name "*.md" | xargs rg "nvidia" -lRepository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 123
🏁 Script executed:
# Check if there are any pip freeze or lock files that might show actual packages
find . -name "*.lock" -o -name "pip*.txt" -o -name "*requirements*.txt"Repository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 111
🌐 Web query:
nvidia-cudnn-cu13 wheel directory structure LD_LIBRARY_PATH site-packages
💡 Result:
After pip install nvidia-cudnn-cu13, the cuDNN shared objects are installed inside your Python environment under site-packages, typically like:
.../lib/pythonX.Y/site-packages/nvidia/cudnn/lib/(containslibcudnn*.so*on Linux)
This is the directory you generally need to add to the dynamic linker search path. [3], [4]
Because the dynamic loader does not search subdirectories recursively, you must add the specific .../nvidia/cudnn/lib directory (not just .../site-packages/nvidia) to LD_LIBRARY_PATH if your runtime doesn’t already discover it. [3]
Practical ways to set it:
- Add just cuDNN:
export LD_LIBRARY_PATH="$(python -c "import nvidia.cudnn,os; print(os.path.join(os.path.dirname(nvidia.cudnn.__file__),'lib'))"):$LD_LIBRARY_PATH"
- Or (common for TF “pip CUDA deps”) add all
site-packages/nvidia/*/libdirs:- Example shown in a TensorFlow install issue where each
.../site-packages/nvidia/<component>/libwas appended toLD_LIBRARY_PATH. [3]
- Example shown in a TensorFlow install issue where each
References:
- Package:
nvidia-cudnn-cu13on PyPI (wheel-based cuDNN runtime distribution). [1] - NVIDIA docs note cuDNN can be installed via pip wheels (
python3 -m pip install nvidia-cudnn-cu13) and that extra host setup may be needed when using cuDNN outside the pip-managed environment. [2]
Sources: [1] [2] [3] [4]
Line 27 has incorrect NVIDIA wheel directory path.
docker/install/install_python_packages.sh installs nvidia-cudnn-cu13, which exposes libraries under site-packages/nvidia/cudnn/lib/, not site-packages/nvidia/cu13/lib/. The configured directory does not exist, causing silent fallback to base image libraries instead of achieving pip-installed library precedence.
Fix
-ENV LD_LIBRARY_PATH="/opt/conda/envs/py312/lib/python3.12/site-packages/nvidia/cu13/lib/:$LD_LIBRARY_PATH"
+ENV LD_LIBRARY_PATH="/opt/conda/envs/py312/lib/python3.12/site-packages/nvidia/cudnn/lib:$LD_LIBRARY_PATH"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker/Dockerfile.mlsys26` around lines 26 - 27, The LD_LIBRARY_PATH entry
set by the ENV LD_LIBRARY_PATH line points to a non-existent pip wheel directory
(site-packages/nvidia/cu13/lib/) so pip-installed nvidia-cudnn-cu13 libs are not
taking precedence; update the path referenced by the ENV LD_LIBRARY_PATH to the
actual wheel install location site-packages/nvidia/cudnn/lib/ so pip-installed
libraries (from the installer script that installs nvidia-cudnn-cu13) are found
before base image libraries and verify the ENV name LD_LIBRARY_PATH remains
unchanged.
| # Configure pip for user-site installations (allows arbitrary users to install packages) | ||
| # This enables 'pip install --user' and 'pip install -e .' to work for any user | ||
| RUN mkdir -p /opt/pip-user && chmod 1777 /opt/pip-user | ||
| ENV PYTHONUSERBASE=/opt/pip-user | ||
| ENV PATH="/opt/pip-user/bin:$PATH" |
There was a problem hiding this comment.
The image still runs as root.
These lines prepare a shared user-site prefix, but there is no USER instruction anywhere in the Dockerfile. That weakens isolation and can fail on platforms that require non-root containers. Please create an unprivileged user and switch to it after the setup steps.
Example hardening change
RUN mkdir -p /opt/pip-user && chmod 1777 /opt/pip-user
ENV PYTHONUSERBASE=/opt/pip-user
ENV PATH="/opt/pip-user/bin:$PATH"
+RUN useradd --create-home --uid 1000 runner && chown -R runner:runner /workspace
+USER runner🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker/Dockerfile.mlsys26` around lines 37 - 41, The Dockerfile prepares
/opt/pip-user and sets PYTHONUSERBASE/PATH but never drops root; create an
unprivileged user (e.g., addgroup/adduser or use groupadd/useradd to make
'appuser' with a fixed uid/gid), chown the /opt/pip-user directory and any other
runtime dirs to that user, and add a USER appuser instruction after the
build/setup steps so the container runs non-root; reference the existing RUN
mkdir -p /opt/pip-user and ENV PYTHONUSERBASE=/opt/pip-user / ENV
PATH="/opt/pip-user/bin:$PATH" when locating where to chown and switch users.
| wget -O Miniforge3.sh "https://github.com/conda-forge/miniforge/releases/latest/download/Miniforge3-$(uname)-$(uname -m).sh" | ||
| bash Miniforge3.sh -b -p $1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n docker/install/install_python.shRepository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 486
Pin Miniforge to a specific release and verify its checksum before execution.
Using the moving latest URL creates non-reproducible builds—different timestamps can pull different Miniforge versions. Additionally, executing the downloaded installer without checksum verification is a supply-chain security gap. Pin a specific release version and verify the SHA256 checksum before running.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker/install/install_python.sh` around lines 9 - 10, Replace the moving
"latest" download and unconditional execution of Miniforge in the install
script: pin a concrete Miniforge release by using its exact release filename/URL
instead of the `latest` path, download that file to a stable name (e.g.,
Miniforge3.sh), compute and verify its SHA256 checksum (compare output of
sha256sum or shasum -a 256 against the expected checksum string) and abort the
script if the checksum does not match, then only run `bash Miniforge3.sh -b -p
$1` after verification; also remove the installer after successful installation
to avoid leaving executables around.
| wget -O Miniforge3.sh "https://github.com/conda-forge/miniforge/releases/latest/download/Miniforge3-$(uname)-$(uname -m).sh" | ||
| bash Miniforge3.sh -b -p $1 | ||
|
|
||
| $1/bin/conda create -n $2 python=3.12 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "install_python.sh" -type f 2>/dev/null | head -5Repository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 68
🏁 Script executed:
git ls-files | grep -E "install.*python" | head -20Repository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 160
🏁 Script executed:
cat -n docker/install/install_python.shRepository: flashinfer-ai/flashinfer-bench-starter-kit
Length of output: 486
🌐 Web query:
conda create non-interactive mode Docker -y flag behavior
💡 Result:
conda create has no special “Docker/non-interactive mode”. To make it non-interactive, you must pre-answer its confirmations.
-y/--yesmakesconda createauto-answer yes to its confirmation prompts (add/remove/backup, “Proceed?”, etc.), so it won’t pause waiting for user input during image builds. [1]- If you want this behavior globally (e.g., across multiple
condacommands in aRUNstep), you can set:CONDA_ALWAYS_YES=true, orconda config --set always_yes true(often done per-env with--env). [2][3]
This only affects confirmation prompts from conda; it doesn’t solve unrelated Docker issues like conda activate not persisting across layers/shells. [1]
Sources: [1] conda create docs, [2] Stack Overflow (CONDA_ALWAYS_YES), [3] Stack Overflow (conda config always_yes).
Add -y to conda create.
This Docker step is non-interactive, so conda create will pause on confirmation prompts and hang the build. Quote the parameters for robustness.
Proposed fix
-$1/bin/conda create -n $2 python=3.12
+"$1/bin/conda" create -y -n "$2" python=3.12📝 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.
| $1/bin/conda create -n $2 python=3.12 | |
| "$1/bin/conda" create -y -n "$2" python=3.12 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker/install/install_python.sh` at line 12, The conda create invocation
"$1/bin/conda create -n $2 python=3.12" will hang in non-interactive Docker
builds; update the command used (the invocation of conda in the installer
script) to run non-interactively by adding the -y flag and quote the variables
for robustness (e.g., quote $1 and $2 and the package spec) so the command
becomes non-interactive and safe with spaces in paths or env names.
Ubospica
left a comment
Ubospica
left a comment
There was a problem hiding this comment.
Build docker
- Manually Build
- Prebuilt image: will be provided later
|
We can use docker image |
|
Changed the PR to draft for now, since we can use docker.io/flashinfer/flashinfer-ci-cu131:latest instead. Will update if we get new/different package requirement |
|
I think it would be great to update the modal script to use the same flashinfer-bench-starter-kit/scripts/run_modal.py Lines 28 to 31 in 0476c41 here, Tested this and works fine. It might be automated via parsing those files directly too. |
|
Issue: CUTLASS/CuTe C++ headers not discoverable by nvcc in the evaluation Docker image Hi, our latest submission which uses CuTe C++ headers directly from a CUDA kernel ( This path is not in nvcc's default include search path. When We verified this on Modal using the same Docker image:
Suggested fix (any one of):
This would allow CUDA solutions to use CuTe/CUTLASS C++ APIs directly — complementing the existing Python CuTe DSL support. Thanks! |
Could you check if the workaround below works for you? |
|
@yongwww It works on my side. Should I add this env var to my local eval scripts, or will it be added to the official eval environment? |
4 participants
cc: @yzh119 @Ubospica
Summary by CodeRabbit