-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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.
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
Setup model routing config and plan routing to o1 #6189
base: main
Are you sure you want to change the base?
Changes from 13 commits
cec10a1
fd1b4ec
c33ba45
7b08724
910ba8c
b73f3ec
54d4401
06db2d6
b5973cd
e3c8a9e
27a83db
6f86ad9
ec2d162
9bf5a7f
8e05f3f
ddc3248
472d95c
fc5040c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
from dataclasses import dataclass, fields | ||
|
||
from openhands.core.config.config_utils import get_field_info | ||
|
||
|
||
@dataclass | ||
class ModelRoutingConfig: | ||
reasoning_model: str = 'o1-preview-2024-09-12' | ||
|
||
def defaults_to_dict(self) -> dict: | ||
"""Serialize fields to a dict for the frontend, including type hints, defaults, and whether it's optional.""" | ||
dict = {} | ||
for f in fields(self): | ||
dict[f.name] = get_field_info(f) | ||
return dict | ||
|
||
def __str__(self): | ||
attr_str = [] | ||
for f in fields(self): | ||
attr_name = f.name | ||
attr_value = getattr(self, f.name) | ||
|
||
attr_str.append(f'{attr_name}={repr(attr_value)}') | ||
|
||
return f"ModelRoutingConfig({', '.join(attr_str)})" | ||
|
||
@classmethod | ||
def from_dict(cls, model_routing_config_dict: dict) -> 'ModelRoutingConfig': | ||
return cls(**model_routing_config_dict) | ||
|
||
def __repr__(self): | ||
return self.__str__() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ | |
|
||
import requests | ||
|
||
from openhands.core.config import LLMConfig | ||
from openhands.core.config import LLMConfig, ModelRoutingConfig | ||
|
||
with warnings.catch_warnings(): | ||
warnings.simplefilter('ignore') | ||
|
@@ -85,6 +85,7 @@ def __init__( | |
self, | ||
config: LLMConfig, | ||
metrics: Metrics | None = None, | ||
model_routing_config: ModelRoutingConfig | None = None, | ||
): | ||
"""Initializes the LLM. If LLMConfig is passed, its values will be the fallback. | ||
|
@@ -100,6 +101,7 @@ def __init__( | |
) | ||
self.cost_metric_supported: bool = True | ||
self.config: LLMConfig = copy.deepcopy(config) | ||
self.model_routing_config = model_routing_config | ||
|
||
self.model_info: ModelInfo | None = None | ||
|
||
|
@@ -158,6 +160,7 @@ def wrapper(*args, **kwargs): | |
|
||
messages: list[dict[str, Any]] | dict[str, Any] = [] | ||
mock_function_calling = kwargs.pop('mock_function_calling', False) | ||
use_reasoning_model = kwargs.pop('use_reasoning_model', False) | ||
|
||
# some callers might send the model and messages directly | ||
# litellm allows positional args, like completion(model, messages, **kwargs) | ||
|
@@ -189,6 +192,15 @@ def wrapper(*args, **kwargs): | |
kwargs['stop'] = STOP_WORDS | ||
mock_fncall_tools = kwargs.pop('tools') | ||
|
||
if use_reasoning_model: | ||
if self.model_routing_config is None: | ||
raise ValueError( | ||
'Model routing config is required for model routing.' | ||
) | ||
|
||
# Replace the model with the reasoning model | ||
kwargs['model'] = self.model_routing_config.reasoning_model | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could design the reasoning model not as a part of an LLM instance, but as a second LLM instance in the agent? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we also need to allow user to set these, especially if they don't use via a llm proxy 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
|
||
# if we have no messages, something went very wrong | ||
if not messages: | ||
raise ValueError( | ||
|
@@ -636,7 +648,7 @@ def __str__(self): | |
return f'LLM(model={self.config.model}, api_version={self.config.api_version}, base_url={self.config.base_url})' | ||
elif self.config.base_url: | ||
return f'LLM(model={self.config.model}, base_url={self.config.base_url})' | ||
return f'LLM(model={self.config.model})' | ||
return f'LLM(model={self.config.model},reasoning_model={self.model_routing_config.reasoning_model if self.model_routing_config else None})' | ||
|
||
def __repr__(self): | ||
return str(self) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
from abc import ABC, abstractmethod | ||
|
||
|
||
class BaseRouter(ABC): | ||
@abstractmethod | ||
def should_route_to_custom_model(self, prompt: str) -> bool: | ||
pass |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
from openhands.router.plan.llm_based import LLMBasedPlanRouter | ||
from openhands.router.plan.rule_based import RuleBasedPlanRouter | ||
|
||
__all__ = ['RuleBasedPlanRouter', 'LLMBasedPlanRouter'] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
import copy | ||
|
||
from openhands.core.config import LLMConfig | ||
from openhands.llm.llm import LLM | ||
from openhands.router.base import BaseRouter | ||
from openhands.router.plan.prompts import ANALYZE_PROMPT | ||
|
||
|
||
class LLMBasedPlanRouter(BaseRouter): | ||
""" | ||
Router that routes the prompt that is judged by a LLM as complex and requires a step-by-step plan. | ||
""" | ||
|
||
JUDGE_MODEL = 'gpt-4o' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be interesting to see if we can experiment with cheaper model for that 🤔 |
||
|
||
def __init__(self, llm_config: LLMConfig): | ||
super().__init__() | ||
|
||
judge_llm_config = copy.deepcopy(llm_config) | ||
self.judge_llm = LLM(judge_llm_config) | ||
|
||
def should_route_to_custom_model(self, prompt: str) -> bool: | ||
messages = [] | ||
|
||
messages.append( | ||
{ | ||
'role': 'user', | ||
'content': ANALYZE_PROMPT.format(message=prompt), | ||
} | ||
) | ||
|
||
response = self.judge_llm.completion( | ||
messages=messages, | ||
model=self.JUDGE_MODEL, | ||
) | ||
return int(response['choices'][0]['message']['content'].strip()) == 1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
ANALYZE_PROMPT = """Analyze this prompt to see if it requires a detailed plan generation. | ||
ryanhoangt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Some example scenarios that require generating a step-by-step plan: | ||
1. Structured Rule-Based Tasks with Well-Defined Constraints | ||
* Example: In a synthetic task, adhering to a sequence like loosening nuts before removing wheels is critical | ||
2. Tasks Requiring Step-by-Step Reasoning to plan a structured chain of actions | ||
* Example: In a synthetic task, objects must be manipulated in a sequence to achieve a configuration | ||
3. Scenarios with Limited Resources or Strict Constraints | ||
* Tasks that require resource-sensitive planning, such as minimizing actions or handling tools efficiently | ||
* Example: In a synthetic task, we need to efficiently coordinate robot actions across rooms and minimize energy consumption costs | ||
4. Generalization in Familiar Symbolic Representations | ||
* Tasks where the rules remain consistent, and the specific instances change. | ||
* Example: When we need to adapt strategies to new but structured instances of tasks. | ||
5. Requests Requiring Self-Evaluation | ||
* Self-evaluation mechanism enables the identification and correction of errors mid-process. | ||
* Example: When we need to reevaluate actions and adjust plans or actions based on constraints. | ||
In context of software engineering, below are some scenarios where plan generation is required: | ||
1. Dependency and Workflow Management | ||
* Automating and optimizing CI/CD pipelines, build processes, and package dependency resolution. | ||
* Example: Resolving complex dependency graphs or sequencing multi-step deployments. | ||
2. Code Refactoring and Debugging | ||
* Planning systematic changes for refactoring large codebases and isolating root causes during debugging. | ||
* Example: Refactoring monolithic code into modular components while preserving functionality. | ||
3. Infrastructure and Resource Planning | ||
* Designing and optimizing Infrastructure as Code (IaC) changes and dynamic resource allocation. | ||
* Example: Planning cloud resource provisioning while adhering to dependency constraints. | ||
4. High-level Requirements to Low-level Implementation Mapping | ||
* Translating high-level requirements into detailed implementation steps and ensuring consistency. | ||
=== BEGIN USER MESSAGE === | ||
{message} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could also experiment sending O1 with the last 5/10 action/observation 🤔 in case there's some deep reasoning required to figure out the error, etc. |
||
=== END USER MESSAGE === | ||
Only respond with 0 for no plan generation required or 1 for plan generation required. | ||
""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
from openhands.router.base import BaseRouter | ||
|
||
|
||
class RuleBasedPlanRouter(BaseRouter): | ||
""" | ||
Router that detects if the prompt contains the string "plan". | ||
""" | ||
|
||
def should_route_to_custom_model(self, prompt: str) -> bool: | ||
# Returns True if the prompt contains the word "plan" | ||
return 'plan' in prompt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this is also another approach, my thought is for now we only use reasoning models specifically for model routing, so I put it in this config group (with other values in the future). When we also use them for other purposes, we can probably move to llm-specific groups?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point is that we can reuse the way we define a model (which will implicitly take care of the correct loading and init all
base_url
etc).It doesn't say which component of
openhands
loads the definition of[llm.reasoning_model]
, it can be the routing component.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify, if a user wants to use a reasoning model today, for the agent, they can do so. They just choose a reasoning model and configure it. Ability to use it isn't new?
We can just avoid to duplicate LLMConfig settings ("reasoning_model", "reasoning_model_base_url", "reasoning_model_api_key", "reasoning_model_aws...something" etc) into the new routing section, instead we can reference existing configurations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that sounds good to me, thanks for the suggestion! I'll try to address this after getting the routing behavior to work