-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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
llguidance build fixes for Windows #11664
base: master
Are you sure you want to change the base?
Conversation
i was hoping to dig a bit more into the code this afternoon, but I've ran out of time before my kids come home from school. But it looks like |
I suspect LlamaSharp is only linking against |
Correct, it only takes a dependency on llama.dll that's shipped with the releases and calls that via .NET's DllImports to do the unmanaged calls to exported functions. So it would need to be exported. I could see an argument that if the build was done with LLGuidance flag set then the existing llama_sampler_init_grammar and llama_sampler_init_grammar_lazy should be able to handle routing the grammar without the need for additional calls exported. much like the set up with |
It was ggerganov decision to include the llguidance only in common.lib, which is not AFAIK included in llama.dll. So the code is not there, it's not only about exporting. I understand he's open to including it in llama.dll at a later date after it proves itself. |
Ok, that makes sense. Then no worries, I think I can come up with some creative solutions until it's properly baked now that it's building on Windows |
Yes, in the future we can include Sent you a collaborator invite - feel free to merge this PR if it is still needed. |
The actual changes is by @phil-scott-78 , see comment
I tested it on fresh Windows install, did minor naming fixes, and updated docs.
Also fixed link to VS download to be English not German.
cc @HanClinto