Fix the incompatibility bug of azure deployment id configuration with gpt4/3-only

[IANTHEREAL]

Background

When I use the azure openai api and set the set gpt4 only, auto gpt will fail because the deployment id is incorrect.

azure.yaml

azure_api_type: azure
azure_api_base: https://test.openai.azure.com/
azure_api_version: 2023-03-15-preview
azure_model_map:
    fast_llm_model_deployment_id: gpt-3-5_playground
    smart_llm_model_deployment_id: gpt-4_playground
    embedding_model_deployment_id: gpt-embedding-ada

min test script

from autogpt.config.config import Config

CFG = Config()
print(CFG.fast_llm_model, CFG.get_azure_deployment_id_for_model(CFG.fast_llm_model))
print(CFG.smart_llm_model, CFG.get_azure_deployment_id_for_model(CFG.smart_llm_model))

CFG.set_smart_llm_model(CFG.fast_llm_model)
print(CFG.fast_llm_model, CFG.get_azure_deployment_id_for_model(CFG.fast_llm_model))
print(CFG.smart_llm_model, CFG.get_azure_deployment_id_for_model(CFG.smart_llm_model))

output

gpt-3.5-turbo -> gpt-3-5_playground
gpt-4 -> gpt-4_playground
gpt-4 -> gpt-3-5_playground
gpt-4 -> gpt-3-5_playground

But it should be

gpt-3.5-turbo -> gpt-3-5_playground
gpt-4 -> gpt-4_playground
gpt-4 -> gpt-4_playground
gpt-4 -> gpt-4_playground

Changes

When the user sets set gpt4 only, modify the corresponding fast llm model related configuration

Documentation

Test Plan

PR Quality Checklist

• My pull request is atomic and focuses on a single change.
• I have thoroughly tested my changes with multiple different prompts.
• I have considered potential risks and mitigations for my changes.
• I have documented my changes clearly and comprehensively.
• I have not snuck in any "extra" small tweaks changes

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
docs	⬜️ Ignored (Inspect)	Visit Preview		Jun 7, 2023 3:16am

[lc0rp]

The underlying issue is addressed in #3144.

@IANTHEREAL Shall we close this and collaborate there?

[IANTHEREAL]

@lc0rp Hi,but I think these two PRs don't solve the same problem

[codecov]

Codecov Report

Patch coverage: 85.71% and project coverage change: -0.06 ⚠️

Comparison is base (9e5492b) 50.04% compared to head (6828d11) 49.98%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4098      +/-   ##
==========================================
- Coverage   50.04%   49.98%   -0.06%     
==========================================
  Files         116      116              
  Lines        4826     4825       -1     
  Branches      650      650              
==========================================
- Hits         2415     2412       -3     
- Misses       2227     2228       +1     
- Partials      184      185       +1     

Impacted Files	Coverage Δ
autogpt/config/config.py	82.55% <83.33%> (-1.35%)	⬇️
autogpt/configurator.py	29.48% <100.00%> (-0.90%)	⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[lc0rp]

@lc0rp Hi, but I think these two PRs don't solve the same problem

Well, not exactly the same, but parts of this PR overlap with #3144 and this PR may be fixed by #3144. So, I should have said they're related. In this PR, you suggest that if we're in gpt4only mode, we should pull settings from the CFG.smart_llm_model and vice-versa.

In PR #3144, we ignore the config when we select an exclusive mode at the command line. It shouldn't matter whether the smart_llm_model is set to 3.5 or 4 - in gpt4only mode, we should use GPT-4.

[lc0rp]

Added a couple of comments. Happy to hop on a discord chat/call.

[IANTHEREAL]

@lc0rp Hi
The crux of the issue we're discussing is that, although the model only consists of GPT3.5 and GPT4, there are multiple parameters that can be set independently. Hence, we are forced to make some assumptions based on expectations.

My background is that I'm develop an online service, and neither errors nor performance degradation are acceptable to me.

For example, when setting 'gpt4only':
1.1. Incorrectly selecting the deployment id due to 'gpt4 only' setting shouldn't occur.
1.2. I'd prefer not to see a trade-off where setting the fast model token limit to 4000 results in smaller memory and a subsequent loss of performance.

The same applies when setting 'gpt3only'. Encountering an error due to the smart model token limit being 8000 is something I'd rather not see.

At present, my modifications have only mitigated these issues, but this isn't the best solution. Incorporating these related parameters into two separate GPT configuration objects will be more nature, thus avoiding these discussions that I see as somewhat compromised.

I apologize if my response is a bit rushed. If you have thoughts, I'd be happy to discuss further on the community discord after I finish a project in the next couple of days.

To add to that, I've just tested your code with gpt4only=True, and it seems that the function get_azure_deployment_id_for_model() isn't working as expected.

[IANTHEREAL]

Proposal for a Structured Configuration Object for GPT Models

In the current implementation of Auto-GPT, there are multiple parameters related to GPT-3 and GPT-4 models that can be set independently. This can lead to unexpected behavior when the deployment ID for the model is fetched using the get_azure_deployment_id_for_model() function. A solution to this problem is to encapsulate all model-related parameters into a structured configuration object, which will both simplify the code and reduce the likelihood of errors.

Proposal

The proposal is to introduce a GPTConfig data class that contains all relevant parameters for a GPT model. Here is a potential implementation:

@dataclass
class GPTConfig:
    model: str
    api_key: str
    token_limit: int = 4000
    temperature: float = 0
    api_deployment_id: Optional[str] = None

With this data class, we can create separate configuration objects for GPT-3 and GPT-4 models from ENV:

gpt3 = GPTConfig(model='gpt-3', api_key='gpt3_api_key', api_deployment_id='gpt-3_deployment_id')
gpt4 = GPTConfig(model='gpt-4', api_key='gpt4_api_key', api_deployment_id='gpt-4_deployment_id')

To manage these configurations, a Config class can be created:

class Config:
    def __init__(self):
        self.model_config = {
            'gpt-3': gpt3,
            'gpt-4': gpt4,
        }
        self.fast_llm_model = 'gpt-3'
        self.smart_llm_model = 'gpt-3'

    def set_fast_llm_model(self, model: str):
        # check whether model is in self.model_config
        if model in self.model_config:
            self.fast_llm_model = model
        else:
            raise ValueError(f"Model {model} is not in the configuration")

Benefits

1. Code Simplification: Encapsulating all model-related parameters into a single data structure simplifies the codebase and makes it easier to understand and maintain. This approach brings clarity to the configuration process by making the relationships between the different parameters explicit. This can make the system easier to debug and extend.

2. Error Reduction: By ensuring that all model-related parameters are set consistently, this approach reduces the likelihood of errors.

3. Enhanced Flexibility: This approach allows for easy configuration of multiple heterogeneous models in the future. If a new model variant is introduced, a new configuration object can be easily created for it.

[IANTHEREAL]

Proposal for a Structured Configuration Object for GPT Models

@lc0rp How do you feel about it? Do you think you could take on the implementation of this proposal? I would really appreciate your assistance with this.

[github-actions]

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

[lc0rp]

Proposal for a Structured Configuration Object for GPT Models

@lc0rp How do you feel about it? Do you think you could take on the implementation of this proposal? I would really appreciate your assistance with this.

Oops, I was absent these past few days. However, I agree with you regarding the matter of multiple independent variables that need to be linked, and I must say, your proposal looks fantastic!

I am pleased to note that the re-architectural design has also reached similar conclusions, and your proposal resembles the re-architectural code. The re-architectural code is situated in a separate branch as all the loose ends have been tied. It is almost ready for release.

Take a look at the LLM base models through this link: https://github.com/Significant-Gravitas/Auto-GPT/blob/agent-state-encapsulation/autogpt/llm/base.py and let me know your thoughts. If you'd like to continue the proposal afterward, I'd be happy to collaborate. We'll move it to a new DRAFT issue and invite feedback before PR submission.

[IANTHEREAL]

@lc0rp Similar to how the static attributes of GPT are preserved in the LLM base models: https://github.com/Significant-Gravitas/Auto-GPT/blob/agent-state-encapsulation/autogpt/llm/base.py, I think we can implement a similar GPT dynamic attributes configuration. I'd like to convert this PR into a draft and try to modify it. If you have any other plans or thoughts, please feel free to share with me at any time.

[lc0rp]

@IANTHEREAL Is this still valid? If so, we're prepping for release v0.4.3. Please resolve conflicts and stand by as we merge.

[github-actions]

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

[netlify]

✅ Deploy Preview for auto-gpt-docs canceled.

Name	Link
🔨 Latest commit	f56ff98
🔍 Latest deploy log	https://app.netlify.com/sites/auto-gpt-docs/deploys/64a78c7e12b0f100081759f4

[IANTHEREAL]

@lc0rp Yes, I have resolved it

[github-actions]

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

[Pwuts]

The config module was overhauled in #4803, causing conflicts with this PR (again). I'm not sure why this PR wasn't included in an earlier release yet, it looks like it was mergeable.

Anyhow, it seems that #4803 also broke support for Azure by removing Config.get_azure_deployment_id_for_model(). Instead of resolving the merge conflicts on this PR, maybe it's easier to make a new PR fixing Azure support altogether in a new PR.

Update: @collijk will look into patching basic Azure support first, and then we can check if the issue addressed by this PR still exists.
Update 2: looks like this PR fixes Azure support: #4875

[github-actions]

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.