Allow to set `model` for separate `submit_prompt` calls to OpenAI

vanna-ai / vanna

🤖 Chat with your SQL database 📊. Accurate Text-to-SQL Generation via LLMs using RAG 🔄.

https://vanna.ai/docs/

MIT License

9.97k stars 737 forks source link

Allow to set `model` for separate `submit_prompt` calls to OpenAI #333

Closed andreped closed 3 months ago

andreped commented 3 months ago

Fixes #332.

Changes

[x] Add support to set both model and engine for submit_prompt events.
[x] Allow changing model for different calls without affecting the base vanna instance.

Seems to work fine. I did a small test below to verify that setting the model with this new API works, but I was unable to run tests. Not sure why. Are you able to test and verify that it runs fine, @zainhoda?

I set the model explicitly when running generate_sql like so:

sql = vn_openai.generate_sql("What are the top 4 customers by sales?", model="gpt-4")

To test that it works I just added two prints into the code itself (not included in PR, just for testing):

Console showing that the model is set and passed appropriately to the completion call:

andreped commented 3 months ago

I ran a test by installing Vanna from my fork like so:

pip install git+https://github.com/andreped/vanna@openai-model-fix

At least that seemed to work fine. Did not crash at least and I passed the model argument like above.

andreped commented 3 months ago

I just now realized that we likely have the same issue with temperature and max_tokens, if users want to change these for separate calls but not affect other users also using the same vanna instance.

I can address this in a separate PR after this one, if of interest, such that we can tie this to an issue. I will make an issue about this after this PR has been merged.

andreped commented 3 months ago

Just observed a minor bug where the wrong model/engine variable was included in the prints. Fixed it now.

andreped commented 3 months ago

@zainhoda Thoughts on this PR?

We are currently using this solution in our development branch, but we want this fixed to move to an updated Vanna version, before moving this to our stable branch.