Add argument for extra EOS token to llm-tool

[DePasqualeOrg]

I added an argument for the extra EOS token to match the Python implementation and also improved the console output during loading.

Maybe we should rethink how llm-tool is implemented, since this solution is a bit unwieldy.

[davidkoski]

What if we just made the properties on ModelConfiguration var? There isn't a particular need for them to be let (I just tend to make things let unless I know of a need to mutate). Then we can let the owner of the structure decide if it is immutable or not:

    /// Additional tokens to use for end of string
    public let extraEOSTokens: Set<String>

This code would just be a couple of lines then.

[DePasqualeOrg]

Intuitively I feel like a model configuration shouldn't need to be changed. It should be static for each model. Maybe we need to rethink how the configurations are generated? Unfortunately I don't have time to delve into this at the moment, but I wanted to mention it, since my solution here isn't very elegant.

[davidkoski]

This isn't changing the registry (it can't change it since they are value types), it is simply overlaying the local extra tokens on the "copy" we are going to use.

By making the properties var (ModelConfiguration and ModelContext) it would turn this code into roughly:

var context = context
if let extraEosToken {
    context.configuration.extraEOSTokens.insert(extraEosToken)
}

I think that would be very clean at expressing what we are trying to do (and is a good use of value types).

[DePasqualeOrg]

Okay, your call.

[DePasqualeOrg]

@davidkoski, you can go ahead and change ModelConfiguration in this PR or in a separate PR, whichever you prefer.

[davidkoski]

pushed commit -- I think that captures what you were doing and is a bit simpler in implementation now that it is mutable

look good?

[DePasqualeOrg]

That's definitely more concise. Thank you!

[davidkoski]

Looks good, thanks for the addition!

[davidkoski]

I think we are ready to merge unless you want to add anything.