feat(llm): add support for fine-tuned OpenAI models #682
Conversation
WalkthroughThe changes primarily focus on enhancing the handling of fine-tuned models in the OpenAI API. They introduce a mechanism to extract the "standard" model name from a fine-tuned model, ensuring the correct method is called based on the model name. Additionally, a new test case is added to verify this functionality. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (3)
- pandasai/helpers/openai_info.py (2 hunks)
- pandasai/llm/openai.py (1 hunks)
- tests/llms/test_openai.py (1 hunks)
Files skipped from review due to trivial changes (1)
- pandasai/helpers/openai_info.py
| if "ft:" in self.model: | ||
| # extract "standard" model name from fine-tuned model | ||
| model_name = self.model.split(":")[1] | ||
| else: | ||
| model_name = self.model |
There was a problem hiding this comment.
The code checks if the model name contains "ft:" and extracts the base model name accordingly. This is a good approach to handle fine-tuned models. However, it assumes that the model name after "ft:" is always valid. It would be better to add a check to ensure that the extracted model name is not empty or invalid. This will prevent potential errors when calling the OpenAI API.
if "ft:" in self.model:
# extract "standard" model name from fine-tuned model
model_name = self.model.split(":")[1]
+ if not model_name:
+ raise ValueError("Invalid fine-tuned model name.")
else:
model_name = self.modelCommittable suggestion (Beta)
| if "ft:" in self.model: | |
| # extract "standard" model name from fine-tuned model | |
| model_name = self.model.split(":")[1] | |
| else: | |
| model_name = self.model | |
| if "ft:" in self.model: | |
| # extract "standard" model name from fine-tuned model | |
| model_name = self.model.split(":")[1] | |
| if not model_name: | |
| raise ValueError("Invalid fine-tuned model name.") | |
| else: | |
| model_name = self.model |
| def test_call_finetuned_model(self, mocker, prompt): | ||
| openai = OpenAI(api_token="test", model="ft:gpt-3.5-turbo:my-org:custom_suffix:id") | ||
| mocker.patch.object(openai, "chat_completion", return_value="response") | ||
|
|
||
| result = openai.call(instruction=prompt) | ||
| assert result == "response" |
There was a problem hiding this comment.
The new test case test_call_finetuned_model is testing the call method with a fine-tuned model. It's good to see that the test is mocking the chat_completion method and asserting the expected response. However, it would be beneficial to also verify that the chat_completion method is called with the correct arguments, especially the model name. This will ensure that the call method is correctly extracting the "standard" model name from the fine-tuned model and passing it to the chat_completion method.
- mocker.patch.object(openai, "chat_completion", return_value="response")
+ chat_completion_mock = mocker.patch.object(openai, "chat_completion", return_value="response")- assert result == "response"
+ assert result == "response"
+ chat_completion_mock.assert_called_once_with(instruction=prompt, model="gpt-3.5-turbo")Committable suggestion (Beta)
| def test_call_finetuned_model(self, mocker, prompt): | |
| openai = OpenAI(api_token="test", model="ft:gpt-3.5-turbo:my-org:custom_suffix:id") | |
| mocker.patch.object(openai, "chat_completion", return_value="response") | |
| result = openai.call(instruction=prompt) | |
| assert result == "response" | |
| def test_call_finetuned_model(self, mocker, prompt): | |
| openai = OpenAI(api_token="test", model="ft:gpt-3.5-turbo:my-org:custom_suffix:id") | |
| chat_completion_mock = mocker.patch.object(openai, "chat_completion", return_value="response") | |
| result = openai.call(instruction=prompt) | |
| assert result == "response" | |
| chat_completion_mock.assert_called_once_with(instruction=prompt, model="gpt-3.5-turbo") |
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## main #682 +/- ##
==========================================
- Coverage 83.84% 83.84% -0.01%
==========================================
Files 67 67
Lines 3201 3206 +5
==========================================
+ Hits 2684 2688 +4
- Misses 517 518 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
mspronesti
changed the title
mspronesti
changed the title
|
@mspronesti thanks a lot for the addition, can't wait to play around with it! Merging |
Hi @gventuri,
this PR aims to support OpenAI's newest fine-tuned models. Please notice that I haven't added any dual porting for Azure OpenAI yet as I don't have any access to those (yet).
Summary by CodeRabbit