Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added the ability to add tags to the OAI_CONFIG_LIST, and filter #1226

Merged
merged 4 commits into from
Jan 15, 2024

Conversation

afourney
Copy link
Member

@afourney afourney commented Jan 13, 2024

Why are these changes needed?

Model names may change depending on how they are deployed. For example, "gpt-3.5-turbo" on OpenAI, might be "gpt35_turbo" on Azure, or even something more arbitrary like "mygpt".

In many application scenarios, like the AutoGenBench, example notebooks, etc. we don't really care what the model is called, only that it is of a given family (e.g., GPT-4). Hardcoding the names into the notebooks is prone to errors, requiring people to customize the python itself.

This PR allows one to add a list of tags to each entry in the OAI_CONFIG_LIST, and to filter on these tags. If we standardize on a common set of tags (e.g., using the OpenAI model names), then we can write client software or notebooks, and direct people to add appropriate tags to ensure compatibility with our notebooks without worrying about model name incompatibilities. We can also add standardized tags for different model roles like "compressor", "teacher", "evaluator", etc. and ask that people add those tags to the models they wish to use for those purposes.

For example, we can have a config list that looks like:

 {
        "model": "my_gpt4",
        "tags": ["gpt-4", "gpt4"],
        "api_key": "xxxyyyzzz",
        "base_url": "https://someurl.openai.azure.com/",
        "api_type": "azure",
        "api_version": "2023-07-01-preview"
    }

Then we can select it like this:

config_list = autogen.config_list_from_json(
    "OAI_CONFIG_LIST",
    filter_dict={"tags":["gpt-4"]},
)

Related issue number

#970

Checks

@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1565795) 31.60% compared to head (7efdab8) 50.89%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1226       +/-   ##
===========================================
+ Coverage   31.60%   50.89%   +19.29%     
===========================================
  Files          32       32               
  Lines        4389     4393        +4     
  Branches     1024     1080       +56     
===========================================
+ Hits         1387     2236      +849     
+ Misses       2892     1951      -941     
- Partials      110      206       +96     
Flag Coverage Δ
unittests 50.83% <100.00%> (+19.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Co-authored-by: Chi Wang <wang.chi@microsoft.com>
Copy link
Collaborator

@maxim-saplin maxim-saplin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the PR is good for what it is - add the ability to tag individual models and filter them using those tags.

Yet you touch on a subject of Azure deployment names and indeed it can be anything which creates confusion when you try to map Azure model to a specific OpenAI model (in order to understand token price or context window size). And I don't think that tags alone solve this mapping issues.

E.g. if I have a model with deployment name "gpt4x128" as in the screen below, then OAI_CONFIG_LIST will need to have "model" set to "gpt4x128". And this will break cost accounting as there's no such entry in OAI_PRICE1K.

image

Tags can be used as a lookup key in OpanAIWrapper.cost() function to fix cost accounting for Azure Models with names that do not follow OpenAI model names. Yet it feels a bit of workaround/side-effect and breaking single-responsibility/separation of concerns.

Long story short, tags solve the issues of arranging long config lists of models, yet they don't help with Azure Models mapping to OpenAI models. And it might be reasonable to revisit the whole idea of OAI_PRICE1K and openai_utils.

E.g. look at few more scenarios: Azure models might get different prices at some point, metadata which now only includes costs might be extended with context window limits, there might be a more explicit way for the client code to define model costs/token limits in configs etc.

@afourney
Copy link
Member Author

afourney commented Jan 13, 2024

@maxim-saplin I agree with your assessment. When I set out to write this PR, I actually included additional fields:

"model_family"
"cost_1k_input"
"cost_1k_output"
"context_window"
"max_output_tokens"

We could then filter on them, and use them in downstream calculations.

Probably there are additional relevant metadata once you get into multi-model models.

However, I opted to keep it simple for this PR -- it felt odd to mix metadata with configuration parameters and to deviate so much from the OpenAI client and create **kwargs. So I figured I would start small.

@ekzhu might have some opinions here. I know he was concerned about having a stricter schema for the config_list, and maybe using pydantic. It's possible we can solve several problems at once here, if we choose to make the model configuration a proper "type".

@sonichi sonichi added this pull request to the merge queue Jan 15, 2024
Merged via the queue into main with commit e6325a4 Jan 15, 2024
82 of 87 checks passed
@sonichi sonichi deleted the config_list_tags branch January 15, 2024 05:19
joshkyh pushed a commit that referenced this pull request Jan 17, 2024
* Added the ability to add tags to the OAI_CONFIG_LIST, and filter on them.

* Update openai_utils.py

Co-authored-by: Chi Wang <wang.chi@microsoft.com>

---------

Co-authored-by: Chi Wang <wang.chi@microsoft.com>
whiskyboy pushed a commit to whiskyboy/autogen that referenced this pull request Apr 17, 2024
…rosoft#1226)

* Added the ability to add tags to the OAI_CONFIG_LIST, and filter on them.

* Update openai_utils.py

Co-authored-by: Chi Wang <wang.chi@microsoft.com>

---------

Co-authored-by: Chi Wang <wang.chi@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
oai_config_list Issues relating to OAI_CONFIG_LIST handling.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants