Skip to content
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

Add dummy nvidia headers for offline builds #203

Closed
wants to merge 1 commit into from

Conversation

stephen-huan
Copy link

@stephen-huan stephen-huan commented Dec 24, 2024

Fixes #202. Based on my read of download_and_copy, specifically the usage of shutil.copytree and shutil.copy, this should be safely replaced by the actual nvidia headers once they are downloaded, but I have not actually tested this.

triton-cpu/python/setup.py

Lines 297 to 330 in daa7eb0

def download_and_copy(name, src_path, dst_path, variable, version, url_func):
if is_offline_build():
return
triton_cache_path = get_triton_cache_path()
if variable in os.environ:
return
base_dir = os.path.dirname(__file__)
system = platform.system()
try:
arch = {"x86_64": "64", "arm64": "aarch64", "aarch64": "aarch64"}[platform.machine()]
except KeyError:
arch = platform.machine()
supported = {"Linux": "linux", "Darwin": "linux"}
url = url_func(supported[system], arch, version)
tmp_path = os.path.join(triton_cache_path, "nvidia", name) # path to cache the download
dst_path = os.path.join(base_dir, os.pardir, "third_party", "nvidia", "backend", dst_path) # final binary path
platform_name = "sbsa-linux" if arch == "aarch64" else "x86_64-linux"
src_path = src_path(platform_name, version) if callable(src_path) else src_path
src_path = os.path.join(tmp_path, src_path)
download = not os.path.exists(src_path)
if os.path.exists(dst_path) and system == "Linux" and shutil.which(dst_path) is not None:
curr_version = subprocess.check_output([dst_path, "--version"]).decode("utf-8").strip()
curr_version = re.search(r"V([.|\d]+)", curr_version).group(1)
download = download or curr_version != version
if download:
print(f'downloading and extracting {url} ...')
file = tarfile.open(fileobj=open_url(url), mode="r|*")
file.extractall(path=tmp_path)
os.makedirs(os.path.split(dst_path)[0], exist_ok=True)
print(f'copy {src_path} to {dst_path} ...')
if os.path.isdir(src_path):
shutil.copytree(src_path, dst_path, dirs_exist_ok=True)
else:
shutil.copy(src_path, dst_path)

triton-cpu/python/setup.py

Lines 327 to 330 in daa7eb0

if os.path.isdir(src_path):
shutil.copytree(src_path, dst_path, dirs_exist_ok=True)
else:
shutil.copy(src_path, dst_path)

shutil.copytree

If dirs_exist_ok is true, the copying operation will continue if it encounters existing directories, and files within the dst tree will be overwritten by corresponding files from the src tree.

shutil.copy

If dst specifies a file that already exists, it will be replaced. Returns the path to the newly created file.

Opening here instead of triton upstream because it's most relevant to the cpu backend and there probably won't be any merge conflicts (as third_party/nvidia/backend/include is ignored upstream).

cc @pawelszczerbuk

New contributor declaration

  • I am not making a trivial change, such as fixing a typo in a comment.

  • I have written a PR description following these
    rules.

  • I have run pre-commit run --from-ref origin/main --to-ref HEAD.

  • Select one of the following.

    • I have added tests.
      • /test for lit tests
      • /unittest for C++ tests
      • /python/test for end-to-end tests
    • This PR does not need a test because this is a build change.
  • Select one of the following.

    • I have not added any lit tests.
    • The lit tests I have added follow these best practices,
      including the "tests should be minimal" section. (Usually running Python code
      and using the instructions it generates is not minimal.)

@minjang
Copy link
Collaborator

minjang commented Dec 25, 2024

Like the previous PR, this needs to go to the upstream. You are updating the nvidia backend.

@stephen-huan
Copy link
Author

Opened triton-lang#5492 upstream.

But in this specific circumstance, I doubt there will ever be a merge conflict as (1) these files are not actual Triton source code, but intended to be replaced by the downloaded cuda libraries by setup.py in the course of an ordinary build and (2) accordingly, they are .gitignore'd both here and upstream.

triton-cpu/.gitignore

Lines 59 to 61 in daa7eb0

# Third-party include
third_party/nvidia/backend/include
third_party/nvidia/backend/lib/cupti

@minjang
Copy link
Collaborator

minjang commented Dec 25, 2024

Once your PR is merged in the upstream, after our periodic rebasing, we will eventually get this PR. You don't need to make duplicate PRs. So, let me close it as this PR doesn't seem like an urgent fix.

@minjang minjang closed this Dec 25, 2024
@stephen-huan stephen-huan deleted the offline-build branch December 25, 2024 08:02
@stephen-huan
Copy link
Author

Understood, thanks for your patience!

ThomasRaoux pushed a commit to triton-lang/triton that referenced this pull request Dec 27, 2024
<!---
The core Triton is a small number of people, and we receive many PRs
(thank
you!).  To help us review your code more quickly, **if you are a new
contributor (less than 3 PRs merged) we ask that you complete the
following
tasks and include the filled-out checklist in your PR description.**

Complete the following tasks before sending your PR, and replace `[ ]`
with
`[x]` to indicate you have done them.
-->
#4085 now makes it impossible
to build Triton without downloading cuda libraries (see
#5449). This PR remove dependencies to cuda headers, making
it possible to build a functional Triton without downloading cuda for
backends that don't require cuda at runtime (e.g. amd, cpu).

Fixes #5449. See also
triton-lang#202,
triton-lang#203.

# New contributor declaration
- [x] I am not making a trivial change, such as fixing a typo in a
comment.

- [x] I have written a PR description following these
  [rules](https://cbea.ms/git-commit/#why-not-how).

- [x] I have run `pre-commit run --from-ref origin/main --to-ref HEAD`.

- Select one of the following.
  - [ ] I have added tests.
    - `/test` for `lit` tests
    - `/unittest` for C++ tests
    - `/python/test` for end-to-end tests
  - [x] This PR does not need a test because build system change.

- Select one of the following.
  - [x] I have not added any `lit` tests.
- [ ] The `lit` tests I have added follow these [best
practices](https://mlir.llvm.org/getting_started/TestingGuide/#filecheck-best-practices),
including the "tests should be minimal" section. (Usually running Python
code
    and using the instructions it generates is not minimal.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Building on CPU with only CPU libraries
2 participants