camel-ai/camel

[BUG] O1 Preview Model Will Raise `KeyError`

Closed this issue · 3 comments

Required prerequisites

What version of camel are you using?

0.2.2

Problem description

For o1 model, each time run() is called, it will remove the unsupported keys from the model config dict. However, the run() will be called for multiple times and the second time this will raise a KeyError since the corresponding keys are already deleted.

# o1-preview and o1-mini have Beta limitations
# reference: https://platform.openai.com/docs/guides/reasoning
if self.model_type in [ModelType.O1_MINI, ModelType.O1_PREVIEW]:
# Remove system message that is not supported in o1 model.
messages = [msg for msg in messages if msg.get("role") != "system"]
# Remove unsupported parameters and reset the fixed parameters
del self.model_config_dict["stream"]
del self.model_config_dict["tools"]
del self.model_config_dict["tool_choice"]
self.model_config_dict["temperature"] = 1.0
self.model_config_dict["top_p"] = 1.0
self.model_config_dict["n"] = 1.0
self.model_config_dict["presence_penalty"] = 0.0
self.model_config_dict["frequency_penalty"] = 0.0
response = self._client.chat.completions.create(
messages=messages,
model=self.model_type.value,
**self.model_config_dict,
)
return response

Reproducible example code

from camel.agents import ChatAgent
from camel.configs import ChatGPTConfig
from camel.messages import BaseMessage
from camel.models import ModelFactory
from camel.types import ModelPlatformType, ModelType

model = ModelFactory.create(
    ModelPlatformType.OPENAI,
    model_type=ModelType.O1_PREVIEW,
    model_config_dict=ChatGPTConfig().as_dict(),
)
agent = ChatAgent(
    system_message=BaseMessage.make_assistant_message(
        role_name="Helper",
        content="The system message will be ignored for this model.",
    ),
    model=model,
)
resp = agent.step(
    BaseMessage.make_user_message(
        role_name="User", content="What is 1 + 1?"
    )
)
print(resp.msg.content)

resp = agent.step(
    BaseMessage.make_user_message(
        role_name="User", content="What is 2 + 2?"
    )
)
print(resp.msg.content)

Traceback

Traceback (most recent call last):
  File "D:\Documents\camel\examples\ai_society\small_test.py", line 22, in <module>
    resp = agent.step(
  File "D:\Documents\camel\camel\agents\chat_agent.py", line 565, in step
    ) = self._step_model_response(openai_messages, num_tokens)
  File "D:\Documents\camel\camel\agents\chat_agent.py", line 861, in _step_model_response
    response = self.model_backend.run(openai_messages)
  File "D:\Documents\camel\camel\utils\commons.py", line 271, in wrapper
    return func(self, *args, **kwargs)
  File "D:\Documents\camel\camel\models\openai_model.py", line 103, in run
    del self.model_config_dict["stream"]
KeyError: 'stream'

Expected behavior

No response

Additional context

No response

We've seen a lot of bugs related to some behavior that only will cause bugs when an agent call step() for multiple times. This are usually due to the following reason

  • one-time logic are mistakenly added to some place that will be executed for multiple times (very likely to be called from step())
  • Some temporary intrusive operations (like modifying the config dict) are not recovered afterwards, like changing the tool set of an agent when structuring the output.

This problem will usually pass the tests because most of our unit test will only call step() once, which will fail to catch bugs like these.

Therefore, I think in the PR that fix this issue, we should add more unit tests that will call step() multiple times to catch bugs like these. Also, in the future review we should also pay more attention to potential bugs like these. @Wendong-Fan @lightaime

@WHALEEYE Thanks for pointing this out! I agree that adding unit tests that call step() multiple times is a good approach to catch these issues. We should also ensure that any intrusive operations are properly cleaned up afterwards. I'll make sure to pay more attention to this in future reviews.

for the test enhancement one new issue created here #1047