feat(openai-callback): add Azure OpenAI fine-tuned models #694
Conversation
WalkthroughThe changes introduce a new function Changes
TipsChat with CodeRabbit Bot (
|
mspronesti
force-pushed
the
feat/azure-finetune-cost
branch
from
5d8ae62 to
72695fa
Compare
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## main #694 +/- ##
==========================================
+ Coverage 84.80% 85.16% +0.35%
==========================================
Files 70 70
Lines 3547 3552 +5
==========================================
+ Hits 3008 3025 +17
+ Misses 539 527 -12
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
![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 (2)
- pandasai/helpers/openai_info.py (3 hunks)
- tests/helpers/test_openai_info.py (1 hunks)
Additional comments: 5
tests/helpers/test_openai_info.py (2)
60-81: The new test cases for the
test_handler_openaifunction are well-structured and cover a variety of model names, including variations with different token counts and Azure OpenAI models. This will help ensure that theOpenAICallbackHandlercorrectly calculates the cost for these models.136-142: The
test_handler_finetuned_modelfunction tests the handling of fine-tuned model names, which is a good addition to the test coverage. However, the expected cost for the first model name seems to be significantly higher than for the other models. Please verify if this is correct.pandasai/helpers/openai_info.py (3)
42-52: The addition of new model names to the
MODEL_COST_PER_1K_TOKENSdictionary is a good step towards supporting more models. However, ensure that the cost values are accurate and up-to-date.106-106: The use of the
standardize_model_name()function inget_openai_token_cost_for_model()is a good practice as it ensures that the model name is standardized before it is used to calculate the cost. This improves code consistency and readability.137-137: The use of the
standardize_model_name()function in the__call__()method of theOpenAICallbackHandlerclass is a good practice as it ensures that the model name is standardized before it is used to calculate the cost. This improves code consistency and readability.
| @pytest.mark.parametrize( | ||
| "model_name,expected_cost", | ||
| [ | ||
| ("gpt-35-turbo", 0.0035), | ||
| ( | ||
| "gpt-35-turbo-0613", | ||
| 0.0035, | ||
| ), | ||
| ( | ||
| "gpt-35-turbo-16k-0613", | ||
| 0.007, | ||
| ), | ||
| ( | ||
| "gpt-35-turbo-16k", | ||
| 0.007, | ||
| ), | ||
| ("gpt-4", 0.09), | ||
| ("gpt-4-0613", 0.09), | ||
| ("gpt-4-32k", 0.18), | ||
| ("gpt-4-32k-0613", 0.18), | ||
| ], | ||
| ) |
There was a problem hiding this comment.
The test_handler_azure_openai function is almost identical to the test_handler_openai function, and they test the same logic with slightly different model names. To avoid code duplication, consider combining these two functions into one and extending the parameterized inputs to include all the model names.
| def standardize_model_name( | ||
| model_name: str, | ||
| is_completion: bool = False, | ||
| ) -> str: | ||
| """ | ||
| Standardize the model name to a format that can be used in the OpenAI API. | ||
|
|
||
| Args: | ||
| model_name: Model name to standardize. | ||
| is_completion: Whether the model is used for completion or not. | ||
| Defaults to False. | ||
|
|
||
| Returns: | ||
| Standardized model name. | ||
|
|
||
| """ | ||
| model_name = model_name.lower() | ||
| if ".ft-" in model_name: | ||
| model_name = model_name.split(".ft-")[0] + "-azure-finetuned" | ||
| if "ft:" in model_name: | ||
| model_name = model_name.split(":")[1] + "-finetuned" | ||
| if is_completion and ( | ||
| model_name.startswith("gpt-4") | ||
| or model_name.startswith("gpt-3.5") | ||
| or model_name.startswith("gpt-35") | ||
| or "finetuned" in model_name | ||
| ): | ||
| # The cost of completion token is different from | ||
| # the cost of prompt tokens. | ||
| return model_name + "-completion" | ||
| else: | ||
| return model_name | ||
|
|
There was a problem hiding this comment.
The standardize_model_name() function is a good addition to the codebase. It improves code consistency and readability by standardizing the model name for use in the OpenAI API. However, the function could be simplified by using a dictionary to map the model prefixes to their standardized names, which would make the code more maintainable and easier to extend in the future.
|
@mspronesti thanks a lot, great to also support Azure finetuned models! Merging :) |
|
@gventuri Thanks for merging! Just be careful that you are no longer squashing the commits before merging them (check the commit history). If this was not intentional, I suppose you might want to rebase, otherwise just ignore this comment :) |
|
@mspronesti thanks a lot for reporting. That was actually unintentional, thanks for mentioning :) |
|
I think it was also the case for the previous one or two PRs (which are still in the commit history), but I suppose it's no big deal! |
Completion of #682, plus some tests covering the cost computation for both OpenAI and Azure OpenAI - including fine-tuned models.
Summary by CodeRabbit