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.

Sign up for GitHub

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

Jump to bottom

fix: Issue where CodeAct agent was trying to log cost on local llm and throwing Undefined Model execption out of litellm #1666

Merged
merged 6 commits into from
May 10, 2024
Merged

fix: Issue where CodeAct agent was trying to log cost on local llm and throwing Undefined Model execption out of litellm #1666

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6bf6934
fix: Issue where CodeAct agent was trying to log cost on local llm an…
bartshappee May 9, 2024
3b423a8
Merge branch 'main' into fix/codeact_agent_logging_cost_exception
bartshappee May 9, 2024
2f11a65
Review Feedback
bartshappee May 9, 2024
a43663f
Missing None Check
bartshappee May 10, 2024
8c0cbb0
Review feedback and improved error handling
bartshappee May 10, 2024
58e6798
Merge branch 'main' into fix/codeact_agent_logging_cost_exception
rbren May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions agenthub/codeact_agent/codeact_agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
IPythonRunCellObservation,
NullObservation,
)
from opendevin.llm.llm import LLM, completion_cost
from opendevin.llm.llm import LLM
from opendevin.runtime.plugins import (
JupyterRequirement,
PluginRequirement,
Expand Down Expand Up @@ -228,11 +228,9 @@ def step(self, state: State) -> Action:
],
temperature=0.0,
)
cur_cost = completion_cost(completion_response=response)
self.cost_accumulator += cur_cost
logger.info(
f'Cost: {cur_cost:.2f} USD | Accumulated Cost: {self.cost_accumulator:.2f} USD'
)

self.logCost(response)

action_str: str = parse_response(response)
state.num_of_chars += sum(
len(message['content']) for message in self.messages
Expand Down Expand Up @@ -265,3 +263,12 @@ def step(self, state: State) -> Action:

def search_memory(self, query: str) -> List[str]:
raise NotImplementedError('Implement this abstract method')

def logCost(self, response):
cur_cost = self.llm.completion_cost(response)
self.cost_accumulator += cur_cost
logger.info(
'Cost: %.2f USD | Accumulated Cost: %.2f USD',
cur_cost,
self.cost_accumulator,
)
bartshappee marked this conversation as resolved.
Show resolved Hide resolved
26 changes: 24 additions & 2 deletions opendevin/llm/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import litellm
from litellm import completion as litellm_completion
from litellm import completion_cost
from litellm import completion_cost as litellm_completion_cost
from litellm.exceptions import (
APIConnectionError,
RateLimitError,
Expand All @@ -20,7 +20,7 @@
from opendevin.core.logger import opendevin_logger as logger
from opendevin.core.schema import ConfigType

__all__ = ['LLM', 'completion_cost']
__all__ = ['LLM']

DEFAULT_API_KEY = config.get(ConfigType.LLM_API_KEY)
DEFAULT_BASE_URL = config.get(ConfigType.LLM_BASE_URL)
Expand Down Expand Up @@ -178,6 +178,28 @@ def get_token_count(self, messages):
"""
return litellm.token_counter(model=self.model_name, messages=messages)

def completion_cost(self, response):
"""
Calculate the cost of a completion response based on the model. Local models are treated as free.

Args:
response (list): A response from a model invocation.

Returns:
number: The cost of the response.
"""
if (
'localhost' not in self.base_url
and '127.0.0.1' not in self.base_url
and '0.0.0.0' not in self.base_url
):
try:
cost = litellm_completion_cost(completion_response=response)
return cost
except Exception:
logger.warning('Cost calculation not supported for this model.')
Copy link
Contributor

Choose a reason for hiding this comment

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

We should simply ignore the exception if if 'Model not in model_prices_and_context_window. json' in str(e) as we are running local models to not worry about the cost itself else there would be huge warnings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reached out to the LiteLLM team on how to handle this correctly. The whole point of this code is to not log a warning when running local models, only when running remote unknown (from a pricing perspective) models

Copy link
Contributor

@SmartManoj SmartManoj May 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Got it. then we should use a config or environment variable like MISC for the time being to raise this exception, else the user may skip the warning and to avoid huge warnings too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code change that broke local models is already going to introduce 1 line of log per step, this just puts a warning before that line, not drastically impacting logging

return 0.0

def __str__(self):
if self.api_version:
return f'LLM(model={self.model_name}, api_version={self.api_version}, base_url={self.base_url})'
Expand Down