Skip to content

Conversation

@afourney
Copy link
Member

Why are these changes needed?

Defaulting to GPT-4 can lead to unexpected costs -- especially when the default kicks due to typos or simple misconfigurations. This removes the default, causing model misconfigurations to raise errors.

Related issue number

Addresses #1025

Checks

@codecov-commenter
Copy link

codecov-commenter commented Dec 27, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (55b03bf) 30.71% compared to head (2525f94) 62.88%.

Files Patch % Lines
autogen/agentchat/conversable_agent.py 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1072       +/-   ##
===========================================
+ Coverage   30.71%   62.88%   +32.17%     
===========================================
  Files          30       30               
  Lines        4047     4047               
  Branches      916      965       +49     
===========================================
+ Hits         1243     2545     +1302     
+ Misses       2724     1264     -1460     
- Partials       80      238      +158     
Flag Coverage Δ
unittests 62.78% <50.00%> (+32.12%) ⬆️

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.

Copy link
Contributor

@rickyloynd-microsoft rickyloynd-microsoft left a comment

Choose a reason for hiding this comment

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

Makes sense.

@afourney
Copy link
Member Author

afourney commented Dec 27, 2023

@rickyloynd-microsoft It's interesting... the compressible agent tests are failing now because they are reading a "model" attribute from llm_config rather than from the config_list. That's a bug with compressible agents, but previously hidden with this default!

@rickyloynd-microsoft
Copy link
Contributor

@rickyloynd-microsoft It's interesting... the compressible agent tests are failing now because they are reading a "model" attribute from llm_config rather than from the config_list. That's a bug with compressible agents, but previously hidden with this default!

Turn on the lights and the bugs are revealed!

…wise default to gpt-4. Revist after #1073 is addressed.
@afourney
Copy link
Member Author

afourney commented Jan 3, 2024

Tests failing.

Thanks @ekzhu @sonichi

It needed an update after #1110. I've fixed the tests, and everything looks to be passing now.

Copy link
Contributor

@sonichi sonichi left a comment

Choose a reason for hiding this comment

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

line number shift

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

@sonichi sonichi left a comment

Choose a reason for hiding this comment

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

Approving despite the test failure: https://github.com/microsoft/autogen/actions/runs/7418135849/job/20185641305?pr=1072
because it's due to a design flaw in CompressibleAgent: #1073

@sonichi sonichi added this pull request to the merge queue Jan 5, 2024
Merged via the queue into main with commit e5ebdb6 Jan 5, 2024
@sonichi sonichi deleted the remove_default_gpt4 branch January 5, 2024 17:04
whiskyboy pushed a commit to whiskyboy/autogen that referenced this pull request Apr 17, 2024
* Remove GPT-4 as the default model.

* Updated test_compressible_agent to work around a bug that would otherwise default to gpt-4. Revist after microsoft#1073 is addressed.

* Worked around another bug in test_compressible_agent. It seems the config_list was always empty!

* Reverted changes to compressible agent.

* Noted that GPT-4 is the preferred model in the OAI_CONFIG_LIST_sample and README.

* Fixed failing tests after microsoft#1110

* Update OAI_CONFIG_LIST_sample

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants