-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat: support build tabby on windows #948
Conversation
My test env:
Local test: I was suspecting probably because of unclean local development environment, so I setup a window runner on GitHub, see CI test: It's same result as my local testing,
With some expriments and googling, I didn't have much luck, so raise a PR for public discussion. But I do suspect this corruption is due to incorrect build on WSL test: For the change, I also tested in cc: @wsxiaoys |
I haven’t go through all details - but one thing might worth to try is you could build chat example on windows with llama.cpp instructions. As long as it works, we should have a way to make it behave properly in tabby as well |
First of all: I'm really looking forward to the possibility of using tabby under Windows. This would be fantastic and a great help to me. And i if have to admit that i have no insight to the code here - and do not know Rust at all. So please forgive me if my suggestion is nonsense. But from my understanding your main problem is to get llama.cpp build on different OS ? If my assumption is correct: you may want to consider this repo from mozilla: https://github.com/Mozilla-Ocho/llamafile - for future builds? If not: sorry for not keeping my mouth shut. ;-) |
Your understanding is correct. The main problem I faced currently is if I build
I took a quick look at this repo, seems its target is to generate
Don't say that man, any idea/discussion/suggestion is welcome. @ichDaheim |
One workaround I would like to take is to build |
fd1e5f7
to
b3f4816
Compare
If this is acceptable, I think the PR is ready for review. Essentially, I just added conditional compilation settings for windows, so the changes are supposed to be completely back-compatible on *nx platforms. Both build:
succeed on my local laptop. Be aware that this is the first step to enable tabby to build on windows.
I plan to raise 2 more PRs separately:
|
aim itself is opensource and it looks like its implementation is straight forward to port out from it |
b3f4816
to
a2bd0d7
Compare
Please add windows build to release.yml: https://github.com/TabbyML/tabby/blob/main/.github/workflows/release.yml (with cuda on) |
Is it ok that I raise a separate PR to include cuda change + workflow update? |
please rebase against main |
crates/llama-cpp-bindings/build.rs
Outdated
println!(r"cargo:rustc-link-search=native={}\lib\x64", cuda_path); | ||
} else { | ||
println!("cargo:rustc-link-search=native=/usr/local/cuda/lib64"); | ||
} | ||
println!("cargo:rustc-link-lib=cudart"); | ||
println!("cargo:rustc-link-lib=culibos"); |
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.
I did some search work, and compared the cuda doc carefully and found:
Until 12.0.0
, there indeed exists culibos
regardless of os type, it is a thread abstraction layer, as mentioned here: https://docs.nvidia.com/cuda/archive/12.0.0/cusparse/index.html#static-library-support
Since 12.0.1
, in the similar page, there's no such keyword found: https://docs.nvidia.com/cuda/archive/12.0.1/cusparse/index.html#static-library-support
I do suspect this lib has been removed from 12.0.1
and afterwards, that's why I failed to compile on my laptop cause I installed latest cuda tookit (12.3.1
).
We're using 11.7
during the release build, so I think this line should continue to work, even on the new windows settings (although I didn't test locally)
As for local development, we can raise a separate PR to fix it for both windows & *nix platforms. What do you think? @wsxiaoys
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.
as long as the ci build pass - we're good
Whether to fix the build for cuda12 can be discussed separately
6fbbfb5
to
7649b8f
Compare
ci/prepare_build_environment.ps1
Outdated
@@ -0,0 +1,3 @@ | |||
Echo "install protocol buffer compiler..." | |||
choco install protoc | |||
choco install cuda --version=11.7.1.51694 |
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.
One more note, I installed cuda by downloading complete .exe
file (3.05GB) in my laptop, and it installed to C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v12.3
, set CUDA_PATH
by default.
Here I just use choco
and assume it has similar behaviour, but be aware it may cause issue here.
debugging in #1009 |
fixed, please rebase |
c735288
to
e9f1c39
Compare
826d80c
to
e5407da
Compare
e5407da
to
e36aebb
Compare
Otherwise LGTM! |
Thanks for the effort! |
* feat: update config to support build on windows * resolve comment * update release.yml * resolve comment
Hi @darknight @wsxiaoys |
Yes - the distributed binary requires cuda runtime (thus nvidia gpu card). If you're interested in CPU only binary distribution, feel free to file an issue, thanks! |
For #909
The trial is partially successful, raise PR for further discussion.
See comments inline.