-
Notifications
You must be signed in to change notification settings - Fork 470
build: add llama.cpp as submodule. #602
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
Conversation
This will be great! We can then try to make 1 VS solution which compiles the C++ and C# code together with the chosen configuration and with 1 click. |
Can this new VS Solution use the make build scripts included in llama.cpp? I don't really know much about cpp in VS so maybe that integrates easily. Asking because at the moment our build is relatively simple because we just clone the llama.cpp repo and call cmake - no maintenance work is required to keep our build up to date. Don't even need cmake or any other build stuff setup and installed locally, because it's all setup to run in GitHub actions. I'd rather not change either of these properties if possible. |
I think that both solutions could exist next to each other in the future, if you choose to do so. Many people like to compile the C++ code themselves, for example for setting some special properties (e.g., AVX2 + CUDA 12), or adding some C++ code, or for security reasons. But, there are also many who like the simplicity to just download the library and use the code without the need for to recompile the C++ code. |
The issue #599 , if understand is related to that I missed llava_shared DLL on the cuda build, but this should not break anything related with the test because no cuda test are running in CI. The test that fail seems to not be related with llava: |
The same error was happening in #603, not sure why. |
I running the test on the master to try to understand the reason |
If cuda is excluded, is it possible that the newly added model weight leads to a lack of disk space or memory? |
It is simply because of the loaded llava_shared.dll is wrong. If you replace it with the correct dll, then it will run OK. |
Merge this PR without check now since it has no reason for ci to fail because of the submodule. |
I tried in my fork and CI test pass. Some times at the first try sometimes at second try. As an example: I'm aware that we didn´t introduce llava_shared in the CUDA directories and this is something that I will try to add on my next PR, but as far as I'm aware there is no CUDA test on CI. If you are aware of some wrong llava version I would like to know specifically the dll that you are talking about because the DLL theoretically comes from update_binaries pipeline. |
@zsogitbe I've created a new branch experimental_cpp for you. On this branch our review won't be stringent. Good luck for you! |
Thank you Rinne! |
Add llama.cpp as a submodule here for some reason: