-
Notifications
You must be signed in to change notification settings - Fork 331
[Refactor] Refactor some build related configurations #827
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
Changes from 2 commits
17635ac
6248899
d546e39
6595dfc
2c6dc98
5121b7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,24 +2,40 @@ FROM nvidia/cuda:12.1.0-devel-ubuntu18.04 | |
|
|
||
| RUN set -eux; \ | ||
| apt-get update; \ | ||
| apt-get install -y wget curl libtinfo-dev zlib1g-dev libssl-dev build-essential libedit-dev libxml2-dev git; \ | ||
| # Install gcc-9 and g++-9 | ||
| apt-get install -y software-properties-common; \ | ||
| add-apt-repository ppa:ubuntu-toolchain-r/test -y; \ | ||
| apt-get update; \ | ||
| apt-get install -y wget curl libtinfo-dev zlib1g-dev libssl-dev build-essential \ | ||
| libedit-dev libxml2-dev git gcc-9 g++-9; \ | ||
| # Switch default gcc/g++ to new version | ||
| update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-9 100; \ | ||
| update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-9 100; \ | ||
| update-alternatives --install /usr/bin/cc cc /usr/bin/gcc 100; \ | ||
| update-alternatives --install /usr/bin/c++ c++ /usr/bin/g++ 100; \ | ||
| gcc --version; g++ --version; \ | ||
| curl -O https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh; \ | ||
| bash Miniconda3-latest-Linux-x86_64.sh -b -p /miniconda3; \ | ||
| rm Miniconda3-latest-Linux-x86_64.sh | ||
| rm Miniconda3-latest-Linux-x86_64.sh; | ||
|
Comment on lines
3
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To reduce the Docker image size, it's a good practice to clean up the apt cache after installing packages. You can add
Comment on lines
17
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verify Miniconda integrity. Currently the installer isn’t verified. Above diff adds a sha256 check. 🤖 Prompt for AI Agents |
||
|
|
||
| RUN apt-get update && apt-get install -y ninja-build | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| ENV PATH=/miniconda3/bin/:$PATH | ||
|
|
||
| # ✅ Accept Anaconda Terms of Service for both required channels | ||
| RUN conda tos accept --override-channels --channel https://repo.anaconda.com/pkgs/main && \ | ||
| conda tos accept --override-channels --channel https://repo.anaconda.com/pkgs/r | ||
|
|
||
| # Create environments | ||
| RUN set -eux; \ | ||
| conda create -n py38 python=3.8 -y; \ | ||
| conda create -n py39 python=3.9 -y; \ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| conda create -n py310 python=3.10 -y; \ | ||
| conda create -n py311 python=3.11 -y; \ | ||
| conda create -n py312 python=3.12 -y; \ | ||
| ln -s /miniconda3/envs/py38/bin/python3.8 /usr/bin/python3.8; \ | ||
| ln -s /miniconda3/envs/py39/bin/python3.9 /usr/bin/python3.9; \ | ||
| ln -s /miniconda3/envs/py310/bin/python3.10 /usr/bin/python3.10; \ | ||
| ln -s /miniconda3/envs/py311/bin/python3.11 /usr/bin/python3.11; \ | ||
| ln -s /miniconda3/envs/py312/bin/python3.12 /usr/bin/python3.12; \ | ||
| conda install -y cmake patchelf | ||
|
|
||
| WORKDIR /tilelang | ||
| WORKDIR /tilelang | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,11 @@ | ||
| [build-system] | ||
| requires = [ | ||
| "build", | ||
| "cmake>=3.26", | ||
| "packaging", | ||
| "setuptools>=61", | ||
| "torch", | ||
| "wheel", | ||
| "tox", | ||
| "auditwheel", | ||
| "patchelf", | ||
| "ninja", | ||
| "Cython", | ||
| "Cython>=3.0.0", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Build System Dependency InconsistencyRemoving
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainCython 3 floor: verify migration and avoid runtime coupling.
Run to confirm no runtime pin and check for language_level usage: 🏁 Script executed: #!/bin/bash
set -euo pipefail
# 1) Cython must NOT be in runtime reqs
rg -nP '^(Cython|cython)\b' requirements*.txt || true
# 2) Ensure cythonize or language_level is set somewhere
rg -nP '(cythonize\(|language_level\s*=)' -n -S -g '!**/build/**' || true
# 3) Inventory .pyx/.pxd files to scope migration risk
fd -t f -e pyx -e pxd -H || trueLength of output: 421 Remove Cython from runtime requirements; make it build-only and enforce Cython‑3.
🤖 Prompt for AI Agents |
||
| ] | ||
| build-backend = "setuptools.build_meta" | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| # runtime requirements | ||
| Cython | ||
| Cython>=3.0.0 | ||
| numpy>=1.23.5 | ||
| tqdm>=4.62.3 | ||
| typing_extensions>=4.10.0 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,9 @@ | ||
| #pragma once | ||
|
|
||
| #include "gemm_mma.h" | ||
|
|
||
| namespace tl { | ||
| using tl_mma::gemm_rs; | ||
| using tl_mma::gemm_sr; | ||
| using tl_mma::gemm_ss; | ||
| } // namespace tl |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,9 @@ | ||
| #pragma once | ||
|
|
||
| #include "gemm_mma.h" | ||
|
|
||
| namespace tl { | ||
| using tl_mma::gemm_rs; | ||
| using tl_mma::gemm_sr; | ||
| using tl_mma::gemm_ss; | ||
| } // namespace tl |
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.
❓ Verification inconclusive
Base OS and toolchain approach is brittle; prefer CUDA 12.1 on Ubuntu 20.04 and drop PPA.
Ubuntu 18.04 is EOL; PPAs for it are unreliable. CUDA 12.1 images exist for ubuntu20.04, which already provide GCC 9.x so you can skip the PPA and alternatives dance.
Apply this diff to simplify and improve reproducibility:
Check the CUDA base tag and avoid surprises:
🏁 Script executed:
Length of output: 0
Switch base to nvidia/cuda:12.1.0-devel-ubuntu20.04 and drop the gcc PPA/update-alternatives. Ubuntu 18.04 moved to ESM on May 31, 2023; NVIDIA provides CUDA 12.1 devel/runtime images for Ubuntu 20.04 and Focal ships GCC 9 by default — so remove the PPA and simplify installs as in the diff below. (lists.ubuntu.com)
Apply this diff to simplify and improve reproducibility:
📝 Committable suggestion
🤖 Prompt for AI Agents