Support a :max_tokens param in OpenAIValidator.validate_max_tokens #388
Conversation
| @@ -69,7 +69,7 @@ def complete(prompt:, **params) | |||
| return legacy_complete(prompt, parameters) if is_legacy_model?(parameters[:model]) | |||
|
|
|||
| parameters[:messages] = compose_chat_messages(prompt: prompt) | |||
| parameters[:max_tokens] = validate_max_tokens(parameters[:messages], parameters[:model]) | |||
| parameters[:max_tokens] = validate_max_tokens(parameters[:messages], parameters[:model], parameters[:max_tokens]) | |||
There was a problem hiding this comment.
max_tokens = [max_tokens, options[:max_tokens]].minDo you think this ^^ logic should actually live inside of this method and not inside the base_validator or the openai_validator?
For example we could have something like this:
parameters[:max_tokens] = [
parameters[:max_tokens],
validate_max_tokens(parameters[:messages], parameters[:model])
].minI guess what's missing with this approach is raising an error?
There was a problem hiding this comment.
@andreibondarev oh yeah that's leaky, it goes by other key names in other LLMs. My vote is let the validator subclasses handle it (e.g. [options[:max_tokens], super(...)].min). If we did it right in the LLM classes, any individual method calls that use it would lead to some repetition. e.g. OpenAI#chat and #complete would both have similar code. This way the other LLM classes can all pass through their own param names correctly. What do you think?
There was a problem hiding this comment.
@andreibondarev also what error condition did you have in mind? e.g. if :max_tokens is passed in as a non-number value? Or possibly as a negative value?
There was a problem hiding this comment.
@andreibondarev check out the latest commit. I moved it into the OpenAIValidator subclass.
There was a problem hiding this comment.
@andreibondarev also what error condition did you have in mind? e.g. if
:max_tokensis passed in as a non-number value? Or possibly as a negative value?
This is the error that I was referring to: https://github.com/andreibondarev/langchainrb/pull/388/files#diff-79c9e673e647cc360e11c2946b3a4def6a5dfe52480860dc9ccd2101906902a9R30
bricolage
changed the title
|
@bricolage Thank you! |
Motivation
Two of OpenAI's API endpoints (chat and completions) take a
:max_tokensparam, but these are ignored in the current token validation / calculation methodBaseValidator.validate_max_tokens(that method name is slightly misleading as it does more than validate it, but that's for another PR).Changes
Fortunately the changes were minimal.
BaseValidator.validate_max_tokensalready takes an options param, and I just worked on this method in #379. Like in #379 we want to take the lower of two token limits. In this case it's between the max calculated from the leftover tokens andoptions[:max_tokens]. I added two specs for when it's passed in (one for when it's lower, one for when it's higher than the calculated max).Addresses #365