search

microsoft / autogen

A programming framework for agentic AI 🤖
https://microsoft.github.io/autogen/
Creative Commons Attribution 4.0 International
34.13k stars 4.93k forks source link

[Bug]: llm_config = copy.deepcopy(llm_config) causes issue with http_client for proxy setting #2412

Closed tonyaw closed 6 months ago

tonyaw commented 7 months ago

Describe the bug

It is possible to pass a xhttp client to llm_config, like:

import httpx                                                                                                           
client = httpx.Client(verify=False, proxies=None)                                                                      

mixtral_config_list = [                                                                                                
        {
            "model": "mistralai/Mixtral-8x7B-Instruct-v0.1",
            "base_url": "X",
            "api_key": "nopass",                                                                                       
            "http_client": client,
            "max_tokens": 5*1024,
        }
    ]

In this case, you will get following exception:

  File "/home/worker/app/agent/autogen/rag_refine_agents.py", line 220, in __init__
    super().__init__(
  File "/home/worker/app/.venv/lib/python3.11/site-packages/autogen/agentchat/assistant_agent.py", line 62, in __init__
    super().__init__(
  File "/home/worker/app/.venv/lib/python3.11/site-packages/autogen/agentchat/conversable_agent.py", line 143, in __init__
    llm_config = copy.deepcopy(llm_config)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 146, in deepcopy
    y = copier(x, memo)
        ^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
                             ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 146, in deepcopy
    y = copier(x, memo)
        ^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 206, in _deepcopy_list
    append(deepcopy(a, memo))
           ^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 146, in deepcopy
    y = copier(x, memo)
        ^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
                             ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 172, in deepcopy
    y = _reconstruct(x, memo, *rv)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 271, in _reconstruct
    state = deepcopy(state, memo)
            ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 146, in deepcopy
    y = copier(x, memo)
        ^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
                             ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 172, in deepcopy
    y = _reconstruct(x, memo, *rv)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 271, in _reconstruct
    state = deepcopy(state, memo)
            ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 146, in deepcopy
    y = copier(x, memo)
        ^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
                             ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 172, in deepcopy
    y = _reconstruct(x, memo, *rv)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 271, in _reconstruct
    state = deepcopy(state, memo)
            ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 146, in deepcopy
    y = copier(x, memo)
        ^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 231, in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
                             ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/copy.py", line 161, in deepcopy
    rv = reductor(4)
         ^^^^^^^^^^^
TypeError: cannot pickle '_thread.RLock' object

Steps to reproduce

Pass a "http_client" to llm_config.

Model Used

Any model that needs to be accessed with additional http_client configuration.

Expected Behavior

No exception is raised when I use llm_config with http_client.

Screenshots and logs

No response

Additional Information

No response

ekzhu commented 7 months ago

@jackgerrits @davorrunje

AbdurNawaz commented 6 months ago

I found a workaround to skip the llm_config = copy.deepcopy(llm_config) line of code.


import httpx
from autogen.oai.client import OpenAIWrapper
from autogen import AssistantAgent, UserProxyAgent, config_list_from_json                                                                                                           
client = httpx.Client(verify=False, proxies=None)                                                                      

mixtral_config_list = [                                                                                                
        {
            "model": "mistralai/Mixtral-8x7B-Instruct-v0.1",
            "base_url": "X",
            "api_key": "nopass",                                                                                       
            "http_client": client,
            "max_tokens": 5*1024,
        }
    ]

#Setting llm_config to False below skips the the copy.deepcopy() line in at #https://github.com/microsoft/autogen/blob/main/autogen/agentchat/conversable_agent.py#L151
assistant = AssistantAgent("assistant", llm_config=False)
assistant.client = OpenAIWrapper(**llm_config)
user_proxy = UserProxyAgent("user_proxy", code_execution_config={"work_dir": "coding", "use_docker": False})
user_proxy.initiate_chat(assistant, message="Plot a chart of NVDA and TESLA stock price change YTD.")
ekzhu commented 6 months ago

Thanks @AbdurNawaz. Do you want to submit a fix for this in the constructor? We can check for the http_client key and avoid doing deep copy for that value.

cc @jackgerrits to take notice of this usage case for the experimental branch.

AbdurNawaz commented 6 months ago

@ekzhu I'd love to submit a PR - do you think I should add a try/catch around copy.deepcopy and skip it if it fails or just skip the copy if it has http_client?

AbdurNawaz commented 6 months ago

@ekzhu @tonyaw I've created the PR #2579 , pease take a look.

Gr3atWh173 commented 6 months ago

@ekzhu @AbdurNawaz I don't think this warrants adding a special case in Autogen. http_client might as well have some other value which is indeed meant to be cloned.

@tonyaw you can create your own HTTP client class which implements the __copy__ and __deepcopy__ methods, which in this case will just return the client object as is, since it can be shared across threads going by the httpx docs

An HTTP client, with connection pooling, HTTP/2, redirects, cookie persistence, etc. It can be shared between threads.

Something like this:

class ShareOnCloneClient(httpx.Client):
    def __copy__(self):
        return self

    def __deepcopy__(self, memo):
        return self
ekzhu commented 6 months ago

I agree with @Gr3atWh173. @AbdurNawaz please ignore my comment above about checking for http_client. I think adding the __copy__ and __deepcopy__ methods should be the proper way to fix the issue.

tonyaw commented 6 months ago

Yes. ShareOnCloneClient works. The issue can be closed. Thanks for all your great support! :-)