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

cmake: fix CMAKE_CUDA_ARCHITECTURES default #2421

Merged
merged 1 commit into from
Jun 26, 2024
Merged

Conversation

cebtenzzre
Copy link
Member

@cebtenzzre cebtenzzre commented Jun 7, 2024

Because we specify a minimum CMake version of 3.21 in the llmodel project (gpt4all-backend), we get the NEW behavior of CMP0104, which causes enable_langauge(CUDA) to set CMAKE_CUDA_ARCHITECTURES to be set based on nvcc's default.

On my machine this means CMAKE_CUDA_ARCHITECTURES gets set to 52, for my GTX 970. No idea what the v2.8.0 installers were built with, but with this change we can be sure that the next release will have the expected feature support
on 6.1+ architectures.

This is the issue that caused me to accidentally be able to reproduce the PTX incompatibility issue (because I was building for sm_52 but not sm_61), but it does not itself fix the inherent problem with the PTX code shipped with GPT4All v2.8.0 (they were built with too new of a CUDA version for many users, including all Linux users until NVIDIA 555.x comes out of Beta).

Signed-off-by: Jared Van Bortel <jared@nomic.ai>
@cebtenzzre cebtenzzre requested a review from manyoso June 7, 2024 19:43
@ThiloteE
Copy link
Collaborator

ThiloteE commented Jun 11, 2024

I've done tests with Windows 10, AMD 5600 and Nvidia GTX 1060 3GB.
Nvidia driver version 555.99 (32.0.15.5599)
Cuda toolkit: cuda_12.4.1_551.78_windows

👍 2.8.0 Cuda backend does NOT crash.
👎 main with commit 41c9013: Cuda backend DOES crash.
👍 This PR at bd5dfe1: Cuda backend does NOT crash.

Model: Various. E.g.

  • qwen1.5-moe-a2.7b-chat-imat-IQ4_XS.gguf
  • tinyllama-1.1b-chat-v1.0.Q4_0.gguf
  • phi-3 Mini Instruct Q4_0
  • llama-3 Q4_0

@cebtenzzre cebtenzzre merged commit da1823e into main Jun 26, 2024
6 of 19 checks passed
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.

None yet

3 participants