Add model parameter#11
Conversation
|
Might be introducing a bug as same param is used in RecursiveCharacterTextSplitter.from_tiktoken_encoder and OpenAI client call. Checkout tiktoken.model.MODEL_PREFIX_TO_ENCODING and tiktoken.model.MODEL_TO_ENCODING for a supported list of strings to pass. If this change is happening, might as well make it more general and pass a 'translation_agent_model' class / object which implements get_completion and num_tokens_in_string. This way we give flexibility to user in experimenting, model could be local or api based ... etc. |
|
Thank you @liugddx for submitting the PR, and @Omaraldarwish for the extension proposal. @Omaraldarwish I agree that making this more general makes sense. If I understand your suggestion correctly, the "translation_agent_model" class you propose would also potentially give us more flexibility in controlling additional functionality, e.g. the way the reflection is done (which now uses just a call to get_completion()). Or it could be helpful if we wanted to add support for a glossary as described here. |
|
@methanet yes!! I'm actually working on branch that pulls all parameters out so users can explicitly control them / experiment with the them -that's turning out to be quite the rebase from what is in main right now! Also not clear whether functional or object-oriented would be better. Could go both ways. On the flip side, If we want a quick simple "bring your own model" refactor, it seems we only need the user to pass a class that implements get_completion and num_tokens_in_string. Check out the modified example script and source code here. This frees up model, temperature, and system prompt to the user -- p.s. still need to update docs and test suite! |
No description provided.