llama_chat_apply_template best practices?

[phil-scott-78]

Description

I was working on some samples using the Stateless Executor, and I kept running into instances where the results were pretty crummy and they'd go off the rails pretty frequently, running until the token length was hit. See this example:

Googling for results, a lot of things talked about better models, throwing more stop words at it or a more precise token length. But I found that the results when using anything through ChatSession seemed to not have this issue. That made me feel like there was something else at play beyond model configuration. Digging in, it seems like ChatSession is using LlamaTemplate which ultimately call llama_chat_apply_template

I added a manual call to LlamaTemplate to the StatelessModeExecute sample, and I immediately started getting better results as well as a stop to the rambling.

while (true)
{
    await foreach (var text in executor.InferAsync(GetWrappedPrompt(prompt), inferenceParams))
    {
        Console.Write(text);
    }
    Console.ForegroundColor = ConsoleColor.Green;
    prompt = Console.ReadLine();
    Console.ForegroundColor = ConsoleColor.White;
}

string GetWrappedPrompt(string input)
{
    var template = new LLamaTemplate(model.NativeHandle)
    {
        AddAssistant = true
    };
    template.Add("system", "I am a helpful bot, specializing in bear names.");
    template.Add("user", input);
    return PromptTemplateTransformer.ToModelPrompt(template);
}

However!

I tried to apply a similiar fix to Instruct mode chat. Originally it started repeating immediately with the opening prompt, showing obvious signs of the same issue.

Applying the fix resolved the original issue, but then it struggled with the chat. I suspect because my fix was getting intermixed with the history but I haven't had a chance to fully dig in, but it did give me cause for my "fix"

i'm not really sure what best practices should be here. I feel like a fix in the guts of the built-in executors would be the move, but honestly I've only been using Llama for a few days so I'm not sure if people are apply these system prompts manually in their code and a fix like that would result in breaking like I saw in the instruct mode chat. From looking around at other projects, and even the llama.cpp project itself, it seems like the use of llama_chat_apply_template is a bit of a crap shoot.

Btw, i'm a maintainer of Spectre.Console, brought a smile to my face to see it.

[martindevans]

Btw, i'm a maintainer of Spectre.Console, brought a smile to my face to see it.

I love Spectre.Console, I use it in all my CLI projects these days. Thanks for working on it!

I added a manual call to LlamaTemplate to the StatelessModeExecute sample, and I immediately started getting better results as well as a stop to the rambling.

In general LLMs really rely on the correct prompting format and will see significantly worse results without it.

i'm not really sure what best practices should be here. I feel like a fix in the guts of the built-in executors would be the move

LLamaSharp is in a bit of a weird state at the moment. The high level executors (stateless, instruct, interact) were all built before llama.cpp supported templating and have kind of had it "bolted on" through the ChatSession/LlamaTemplate. There's also the BatchedExecutor that I built a while ago which exposes a much more powerful and lower level API, but it's also much more difficult to use.

My opinion is all of the high level things need a major redesign, building individual executors (which would handle things like templating, sampling and context management for you) on top of the BatchedExecutor as the low level LLM engine. That's not something I have time to do myself at the moment though (I'm spending a lot of my christmas break just keeping up with llama.cpp updates!).

If there are any changes you feel could be made to the StatelessExecutor based on your work (e.g. improving the examples to include your improvements) I'd be very happy to review PRs and work with you on getting that merged :)

[phil-scott-78]

Right now for my needs this was my tweak

    /// <summary>
    /// This executor infer the input as one-time job. Previous inputs won't impact on the 
    /// response to current input. Has ability to apply system template to prompts.
    /// </summary>
    private class CustomStatelessExecutor : StatelessExecutor
    {
        private readonly LLamaWeights _weights;

        /// <summary>
        /// This executor infer the input as one-time job. Previous inputs won't impact on the 
        /// response to current input.
        /// </summary>
        public CustomStatelessExecutor(LLamaWeights weights, IContextParams @params, ILogger? logger = null) : base(weights, @params, logger)
        {
            _weights = weights;
        }

        /// <summary>
        /// Should we apply the default model template to the prompt 
        /// </summary>
        public bool ApplyTemplate { get; init; }
        
        /// <summary>
        /// The system message to use with the prompt. Only used when <see cref="ApplyTemplate" /> is true
        /// </summary>
        public string? SystemMessage { get; init; }

        public new IAsyncEnumerable<string> InferAsync(string prompt, IInferenceParams? inferenceParams = null, CancellationToken cancellationToken = default)
        {
            if (ApplyTemplate)
            {
                var template = new LLamaTemplate(_weights.NativeHandle) { AddAssistant = true };
                if (SystemMessage != null) template.Add("system", SystemMessage);

                template.Add("user", prompt);
                prompt = PromptTemplateTransformer.ToModelPrompt(template);
            }
            
            return base.InferAsync(prompt, inferenceParams, cancellationToken);
            
        }
    }
}

Felt like this was the move. Kinda a weird backwards compatibility thing. Don't want to take someone's prompt and automatically apply the template, even though there is like a 99% chance a user probably wants that and doesn't know it yet. We ran into the same issue with interpolated strings and markup with Spectre.Console. We don't escape automatically, and we have a fix, but the fix would silently break any user who was doing things properly without a warning or any evidence until a certain sequence was hit.

I'll take a look at BatchedExecutor. It was a bit over my head when I looked at it originally, but I have a few more evenings of playing around experience now :-)

[martindevans]

That seems like a reasonable change, want to PR it?

[phil-scott-78]

sure, opened #1025. i'm gonna close this too while I'm at it