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

fix: better CUDA detection #61

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

delehef
Copy link
Contributor

@delehef delehef commented Dec 11, 2024

What ❔

This PR improves the detection of the CUDA toolkit under Linux, and the detection of its version under all OSes.

Why ❔

Under Linux, the CUDA detection is hardcoded to look into /usr/local/cuda, which is not guaranteed to be the default installation folder. The new method first looks for the CUDA_PATH env. var., then default to /usr/local/cuda if it is empty.

The current way of finding out the installed CUDA version is to look for a version.json file in the CUDA installation path. This file is not guaranteed to exist. However, nvcc is guaranteed to be installed with the toolkit; so the new method is to parse its output to find the exact version there.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.

@delehef delehef force-pushed the fix/cuda-version-detection branch 2 times, most recently from 3ffdcf3 to 3c409f7 Compare December 19, 2024 12:40
Copy link
Member

@robik75 robik75 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK unfortunately there is no canonical way to determine where CUDA toolkit is installed and what the version of that installation is.
Parts of https://cmake.org/cmake/help/latest/module/FindCUDAToolkit.html can be taken as inspiration.
Every somewhat recent CUDA toolkit installation now includes the version.json file and all the versions that we care about (11.x and up) included it for sure. We do not support older CUDA versions anyway.
Did you encounter a scenario where using version.json did not work for you?

Cargo.toml Outdated Show resolved Hide resolved
crates/cudart-sys/src/utils.rs Outdated Show resolved Hide resolved
@delehef
Copy link
Contributor Author

delehef commented Jan 9, 2025

AFAIK unfortunately there is no canonical way to determine where CUDA toolkit is installed and what the version of that installation is.

That's true, and that's most unfortunate :/

Did you encounter a scenario where using version.json did not work for you?

AFAICT, the version.json file in only guaranteed to exist in manually installed setups. I can say that e.g. for Ubuntu, Debian, and NixOS, it does not appear when installed from the canonical package manager.

@robik75
Copy link
Member

robik75 commented Jan 10, 2025

Hmm, my experience it that installing CUDA manually on Ubuntu following the official CUDA installation procedure (that uses apt with nvidia's package repository) results in version.json being placed in the root of the CUDA installation.
Either way, we can proceed with your proposed changes, as long as the CI works, it should be fine.

@delehef
Copy link
Contributor Author

delehef commented Jan 14, 2025

version.json being placed in the root of the CUDA installation.

Absolutely, but only for the manually installed version. If ones uses apt to install it, then version.json is not present.

Thank you for the feedback, I addressed your comments.

@robik75
Copy link
Member

robik75 commented Jan 14, 2025

The PR branch seems to be out of date with the base branch. Can you please rebase?

@delehef delehef force-pushed the fix/cuda-version-detection branch from ed6c806 to aa59fa9 Compare January 14, 2025 13:58
@delehef
Copy link
Contributor Author

delehef commented Jan 14, 2025

Done

@robik75 robik75 merged commit e902ca4 into matter-labs:main Jan 14, 2025
13 checks passed
Artemka374 pushed a commit that referenced this pull request Jan 17, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.152.11](v0.152.10...v0.152.11)
(2025-01-17)


### Features

* **proof-compression:** better proof compression
([#67](#67))
([f4eac96](f4eac96))


### Bug Fixes

* better CUDA detection
([#61](#61))
([e902ca4](e902ca4))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: zksync-era-bot <[email protected]>
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.

3 participants