Skip to content

Add Windows CUDA llama.cpp release asset#3248

Open
LauraGPT wants to merge 9 commits into
mainfrom
codex/llamacpp-windows-cuda-release
Open

Add Windows CUDA llama.cpp release asset#3248
LauraGPT wants to merge 9 commits into
mainfrom
codex/llamacpp-windows-cuda-release

Conversation

@LauraGPT

Copy link
Copy Markdown
Collaborator

Summary

Follow-up for #3243 after the runtime backend flag landed: add a Windows CUDA prebuilt release asset for the llama.cpp / GGUF runtime.

This keeps the existing portable CPU assets unchanged and adds a dedicated windows-x64-cuda matrix row that:

  • installs CUDA Toolkit 13.2 on the Windows runner
  • builds with -DGGML_CUDA=ON
  • disables optional CUDA features that are unnecessary for this first SenseVoice graph package (GGML_CUDA_FA, NCCL, graphs)
  • documents the resulting funasr-llamacpp-windows-x64-cuda.zip release package and --backend cuda invocation

Validation

  • python -m pytest runtime/llama.cpp/tests/test_release_workflow.py runtime/llama.cpp/tests/test_backend_flags.py -q -> 5 passed
  • git diff --check
  • YAML parse smoke: verified the release workflow has 7 matrix rows and only windows-x64-cuda is CUDA-enabled

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the README.md to document the new Windows CUDA release package and adds a test suite (test_release_workflow.py) to verify the GitHub Actions workflow configuration. The review feedback highlights three main issues: a factual error in the documentation regarding the CUDA version (referencing non-existent CUDA 13.2 instead of 12.x), a brittle test assertion on the exact version of a third-party GitHub Action, and a logic error in a test that reads the workflow file instead of the README.md to assert documentation content.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread runtime/llama.cpp/README.md Outdated
`--backend cpu` remains the default and is what the portable cross-platform
prebuilt binaries use. Tagged runtime releases also publish a
`funasr-llamacpp-windows-x64-cuda.zip` package when the Windows CUDA release job
passes; it requires an NVIDIA driver compatible with CUDA 13.2. A binary built

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

CUDA 13.2 is not an existing CUDA version (the current major version is 12). This should be updated to the actual CUDA version configured in the workflow (e.g., 12.x or 12.2/12.3) to avoid user confusion.

Suggested change
passes; it requires an NVIDIA driver compatible with CUDA 13.2. A binary built
passes; it requires an NVIDIA driver compatible with CUDA 12.x. A binary built

def test_windows_cuda_build_uses_cuda_toolkit_and_flags():
workflow = WORKFLOW.read_text(encoding="utf-8")

assert "Jimver/cuda-toolkit@v0.2.35" in workflow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Asserting the exact version of a third-party GitHub Action makes the test brittle and prone to failure when the action is updated in the workflow. Asserting just the action name is more robust.

Suggested change
assert "Jimver/cuda-toolkit@v0.2.35" in workflow
assert "Jimver/cuda-toolkit" in workflow

Comment on lines +26 to +31
def test_release_notes_explain_cpu_and_cuda_windows_assets():
workflow = WORKFLOW.read_text(encoding="utf-8")

assert "windows-x64-cuda" in workflow
assert "--backend cuda" in workflow
assert "Windows CUDA" in workflow

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The test reads the GitHub Actions workflow file (WORKFLOW) but asserts strings that are documented in the README.md (such as --backend cuda and Windows CUDA). This appears to be a copy-paste error from previous tests. Reading the README.md file instead makes the test correct and meaningful.

Suggested change
def test_release_notes_explain_cpu_and_cuda_windows_assets():
workflow = WORKFLOW.read_text(encoding="utf-8")
assert "windows-x64-cuda" in workflow
assert "--backend cuda" in workflow
assert "Windows CUDA" in workflow
def test_release_notes_explain_cpu_and_cuda_windows_assets():
readme_path = ROOT / "runtime" / "llama.cpp" / "README.md"
readme = readme_path.read_text(encoding="utf-8")
assert "windows-x64-cuda" in readme
assert "--backend cuda" in readme
assert "Windows CUDA" in readme

@LauraGPT

Copy link
Copy Markdown
Collaborator Author

Update on the Windows CUDA asset branch: I pushed b3ab11816d316ed3ec564b5ef1aac8ef69c2e287 to keep the CUDA release job SenseVoice-specific.

Why: the original request in #3243 is for SenseVoiceSmall on Windows CUDA, while the previous matrix row was compiling every llama.cpp runtime tool with CUDA enabled. The new head keeps the same windows-x64-cuda asset but sets build_target: llama-funasr-sensevoice, so CI validates and packages the backend that actually exposes --backend cuda today.

Validation before push:

  • python -m pytest runtime/llama.cpp/tests/test_release_workflow.py runtime/llama.cpp/tests/test_backend_flags.py -q -> 5 passed
  • workflow YAML parse smoke confirms only windows-x64-cuda is CUDA-enabled and its target is llama-funasr-sensevoice
  • git diff --check

I cancelled the old all-target CUDA run and started a fresh branch workflow on this head: https://github.com/modelscope/FunASR/actions/runs/29650577183

@LauraGPT

Copy link
Copy Markdown
Collaborator Author

Addressed the review feedback on head f0b08bcc7ce7d916cd13749b321771b9d5df9eb9.

Changes:

  • README no longer hard-codes a CUDA patch version; it now points users to the CUDA Toolkit version configured by the Windows CUDA release job.
  • The workflow test now checks the CUDA action name without pinning the third-party action version.
  • The release-notes test now reads runtime/llama.cpp/README.md instead of the workflow file.

Local validation on ind-gpu8:

  • python -m pytest runtime/llama.cpp/tests/test_release_workflow.py runtime/llama.cpp/tests/test_backend_flags.py -q -> 5 passed.
  • YAML smoke: 7 matrix rows; only windows-x64-cuda has cuda: true and build_target: llama-funasr-sensevoice.
  • git diff --check passed.

@LauraGPT

Copy link
Copy Markdown
Collaborator Author

Follow-up on head 1b9f41c288c4f618295b850bfd084ce4eed55c8d: synced the formal GitHub Release notes too, so both README and tag-release text describe the Windows CUDA driver requirement via the CUDA Toolkit version configured by the workflow instead of hard-coding a version string.

Validation on ind-gpu8:

  • python -m pytest runtime/llama.cpp/tests/test_release_workflow.py runtime/llama.cpp/tests/test_backend_flags.py -q -> 5 passed.
  • YAML smoke: 7 matrix rows; only windows-x64-cuda has cuda: true and build_target: llama-funasr-sensevoice.
  • git diff --check passed.

@LauraGPT

Copy link
Copy Markdown
Collaborator Author

Optimization follow-up on head 1e1916c6f336841e6f1ebf82b9379fe05e78ada0: the Windows CUDA asset now passes -DCMAKE_CUDA_ARCHITECTURES=75;86;89;120 via a quoted CMake argument instead of relying on CMake/CUDA defaults. This targets common RTX 20/30/40/50 Windows users and should keep the CUDA CI job bounded; README and release notes now state the targeted CUDA architectures.

Validation on ind-gpu8:

  • python -m pytest runtime/llama.cpp/tests/test_release_workflow.py runtime/llama.cpp/tests/test_backend_flags.py -q -> 5 passed.
  • YAML smoke: 7 matrix rows; only windows-x64-cuda has cuda: true, build_target: llama-funasr-sensevoice, and cuda_architectures: 75;86;89;120.
  • git diff --check passed.

@LauraGPT

Copy link
Copy Markdown
Collaborator Author

Follow-up on head 3fa345603f57cbab7d6ebc0198c846387baf114f: the previous Windows CUDA run made steady progress but was still compiling ggml-cuda MMQ template instances after ~40 minutes, so I changed the first prebuilt CUDA asset to a bounded cuBLAS-first build.

Changes:

  • Keep the asset scoped to llama-funasr-sensevoice.
  • Add -DGGML_CUDA_FORCE_CUBLAS=ON to avoid the long MMQ template-instance compile path for this release asset.
  • Target CUDA architecture 86 for the first Windows CUDA prebuilt package, and document that other GPU architectures should build from source until we split/expand CUDA assets.

Validation on ind-gpu8:

  • python -m pytest runtime/llama.cpp/tests/test_release_workflow.py runtime/llama.cpp/tests/test_backend_flags.py -q -> 5 passed.
  • YAML smoke: 7 matrix rows; only windows-x64-cuda has cuda: true, build_target: llama-funasr-sensevoice, cuda_architectures: 86, and -DGGML_CUDA_FORCE_CUBLAS=ON.
  • git diff --check passed.

@LauraGPT

Copy link
Copy Markdown
Collaborator Author

Follow-up on head 388c687b7bd010632bafdb07ea40365984096b47: confirmed from run 29654302091 that GGML_CUDA_FORCE_CUBLAS=ON still defines the runtime path but upstream ggml CMake still compiles the long template-instances/fattn-mma*.cu and template-instances/mmq*.cu source sets. I added a wrapper-side patch after fetching the pinned llama.cpp commit and before add_subdirectory: when GGML_CUDA_FORCE_CUBLAS is enabled, those two template source globs are skipped for this release build.

Validation on ind-gpu8:

  • python -m pytest runtime/llama.cpp/tests/test_release_workflow.py runtime/llama.cpp/tests/test_backend_flags.py -q -> 6 passed.
  • Normal CMake configure smoke passed.
  • cuBLAS configure smoke observed: FunASR CUDA release: skipping ggml CUDA MMQ/fattn-mma template sources.
  • YAML smoke confirms windows-x64-cuda uses cuda_architectures: 86, build_target: llama-funasr-sensevoice, and -DGGML_CUDA_FORCE_CUBLAS=ON.
  • git diff --check passed.

@LauraGPT

Copy link
Copy Markdown
Collaborator Author

Follow-up on head cc45f4c7f81c7a5a8aeebdfda728f7e1ee539717: the prior wrapper patch emitted the intended message, but the replacement did not match because the literal SRCS variable text was expanded by CMake while constructing the replacement strings. Fixed that and extended the source pruning to match the release flags exactly:

  • GGML_CUDA_FA=OFF now keeps template-instances/fattn-* out of the fetched ggml-cuda source list.
  • GGML_CUDA_FORCE_CUBLAS=ON keeps template-instances/mmq*.cu and template-instances/mmf*.cu out of the fetched source list.

Validation on ind-gpu8:

  • python -m pytest runtime/llama.cpp/tests/test_release_workflow.py runtime/llama.cpp/tests/test_backend_flags.py -q -> 6 passed.
  • cuBLAS configure smoke observed the pruning message.
  • Direct inspection of the fetched patched ggml/src/ggml-cuda/CMakeLists.txt confirms fattn-* sources are under if (GGML_CUDA_FA), and mmq/mmf sources are under if (NOT GGML_CUDA_FORCE_CUBLAS).
  • git diff --check passed.

@LauraGPT

Copy link
Copy Markdown
Collaborator Author

Follow-up on the Windows CUDA failure in run 29655977963:

  • The failed job linked fattn.obj / mmq.obj / mmf.obj against template instantiations that my previous pruning patch had removed.
  • Reverted that ggml CUDA source-list patch so the release build keeps the upstream CUDA source set intact.
  • Kept the safer build-time bounds that are already validated locally: SenseVoice-only target, CUDA architecture 86, cuBLAS path, and a 90-minute timeout for the CUDA matrix row.

Local validation on ind-gpu8:

  • python -m pytest runtime/llama.cpp/tests/test_release_workflow.py runtime/llama.cpp/tests/test_backend_flags.py -q -> 5 passed
  • workflow YAML parse -> ok
  • regular CMake configure smoke -> ok
  • git diff --check -> ok

Manual workflow dispatch is running at https://github.com/modelscope/FunASR/actions/runs/29656771776.

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.

1 participant