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

October binary update #940

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

martindevans
Copy link
Member

@martindevans martindevans commented Oct 4, 2024

Updated binaries to llama.cpp c35e586ea57221844442c65a1172498c54971cb0, built with https://github.com/SciSharp/LLamaSharp/actions/runs/10984319249

This is an unusually large change due to a redesign of the llama.cpp sampling API. The new sampling system has all been exposed in a safe way except for custom samplers, which are not implemented and will need to be tackled in a future update.

Assorted notable changes:

  • Mirostat and Mirostat2 pipeline classes have been removed (they can easily be re-implemented if necessary)
  • Removed all of the IInferenceParams sampling parameters, this must be done through a pipeline
  • Sampling parameters in WebUI are commented out (removal of IInferenceParams properties broke it)
  • Guidance is not supported by llama.cpp, deleted the guidance example
  • Removed our entire GBNF grammar parser, that's now part of llama.cpp
  • Sampling no longer needs to allocate a large temporary array for every token sampled 🥳

Testing:

  • Windows CPU
  • Windows CUDA
  • Windows Vulkan
  • Linux CPU
  • Linux CUDA
  • Linux Vulkan
  • MacOS CPU
  • MacOS Metal

@m0nsky
Copy link
Contributor

m0nsky commented Oct 5, 2024

Test application is running fine on:

  • Windows Vulkan
  • Linux CPU
  • Linux CUDA
  • Linux Vulkan

I noticed seed was removed from the ModelParams, how can we set the seed after this PR?

Edit
I see there's a SetSeed() in LLamaContext. I tried using context.SetSeed(1337); but it throws System.AggregateException: One or more errors occurred. (Unable to find an entry point named 'llama_set_rng_seed' in DLL 'llama'.)

Comment on lines +212 to +218
public void AddDistributionSampler(uint seed)
{
llama_sampler_chain_add(this, llama_sampler_init_dist(seed));

[DllImport(NativeApi.libraryName, CallingConvention = CallingConvention.Cdecl)]
static extern IntPtr llama_sampler_init_dist(uint seed);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Individual samplers which need a seed now accept a seed parameter just for that sampler. For example llama_sampler_init_dist(uint seed);.

@martindevans
Copy link
Member Author

I noticed seed was removed from the ModelParams, how can we set the seed after this PR?

The seed is now set per-sampler, see my other comment for an example.

I'm glad you mentioned this though, because I didn't pass that through to the higher level! I'll add a Seed property to DefaultSamplingPipeline, which is the more direct replacement for ModelParams.

there's a SetSeed() in LLamaContext

Good catch, that method should have been removed too.

 - Exposed `Seed` property on `DefaultSamplingPipeline`
 - Added penalties to `DefaultSamplingPipeline`
 - Added sensible defaults (copied from llama.cpp "main" example) for `DefaultSamplingPipeline`
@m0nsky
Copy link
Contributor

m0nsky commented Oct 5, 2024

Can confirm the Seed in the DefaultSamplingPipeline now works as expected!

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.

2 participants