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.

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

[Proposal] Weak model proposal #1363

Closed
KennyDizi opened this issue Nov 15, 2024 · 9 comments
Closed

[Proposal] Weak model proposal #1363

KennyDizi opened this issue Nov 15, 2024 · 9 comments

Comments

@KennyDizi
Copy link
Contributor

KennyDizi commented Nov 15, 2024

In some cases, e.g., generate PR description, PR update change log, PR add docs, PR generates labels,... the model like: claude-3-5-haiku-20241022, gpt-4o-mini-2024-07-18, ... are intelligent enough to take on the requirements. This is the cheapest and fastest version of its provider. Therefore, I suggest a solution to this idea. It looks like this:

  • Define the weak model
class ModelType(str, Enum):
    REGULAR = "regular"
    TURBO = "turbo"
    WEAK = "weak"
  • Add model_weak to the configuration section, for example:
[config]
model_weak="anthropic/claude-3-5-haiku-20241022"
model="gpt-4o-2024-08-06"
model_turbo="anthropic/claude-3-5-sonnet-20241022"
fallback_models=["gpt-4o-2024-08-06", "anthropic/claude-3-5-sonnet-20241022"]
  • In the flows where we want to use the cheap LLM model, we can point it out when we call the function retry_with_fallback_models, like this: await retry_with_fallback_models(self._prepare_prediction, model_type=ModelType.WEAK)
  • Update function _get_all_models to perform get all models
def _get_all_models(model_type: ModelType = ModelType.REGULAR) -> List[str]:
    if model_type == ModelType.TURBO:
        model = get_settings().config.model_turbo
    elif model_type == ModelType.WEAK:
        model = get_settings().config.model_weak
    else:
        model = get_settings().config.model
    fallback_models = get_settings().config.fallback_models
    if not isinstance(fallback_models, list):
        fallback_models = [m.strip() for m in fallback_models.split(",")]
    all_models = [model] + fallback_models
    return all_models
@KennyDizi
Copy link
Contributor Author

Hi @mrT23, Please take a look at this proposal. If it's reasonable, I would like to open a PR to do so.

@mrT23
Copy link
Collaborator

mrT23 commented Nov 20, 2024

@KennyDizi thanks for the idea.

the original intention of the 'turbo' model was supposed to be like this, but during the process it turned out to be something a bit different.

I am trying to understand the proposed code, and I am not sure I understand it.
Let's imagine we have only 2 models: regular, and weak
shouldn't it be something like:

def _get_all_models(model_type: ModelType = ModelType.REGULAR) -> List[str]:
    if model_type == ModelType.WEAK and get_settings().config.model_weak:
        model = get_settings().config.model_weak
    else:
        model = get_settings().config.model
    fallback_models = get_settings().config.fallback_models
    if not isinstance(fallback_models, list):
        fallback_models = [m.strip() for m in fallback_models.split(",")]
    all_models = [model] + fallback_models
    return all_models

meaning if we define some operation as one that can accept a weak model, we will look first for a weak model, If defined.

what do you think about this logic ?

@KennyDizi
Copy link
Contributor Author

@mrT23 hi, that's a good one.
In general, my idea is:

  • In pr description, pr generate labels, pr update change log, we can use the ast and cheap model like: claude-3-5-haiku-20241022, gpt-4o-mini-2024-07-18
  • In the pr review, pr code suggestion, we can use the regular model or turbo model based on our choice.

@KennyDizi
Copy link
Contributor Author

By the way this PR description has written by model anthropic/claude-3-5-haiku-20241022

@mrT23
Copy link
Collaborator

mrT23 commented Dec 2, 2024

@KennyDizi my suggestion is this:

  • let's remove the 'turbo' model, and instead introduce a 'weak' model configuration
  • for commands that explicitly allow usage of weak model, if one is defined, we will use it instead as the default
def _get_all_models(allow_weak_model=False) -> List[str]:
    if allow_weak_model and get_settings().config.model_weak:
        model = get_settings().config.model_weak
    else:
        model = get_settings().config.model
    fallback_models = get_settings().config.fallback_models
    if not isinstance(fallback_models, list):
        fallback_models = [m.strip() for m in fallback_models.split(",")]
    all_models = [model] + fallback_models
    return all_models

@KennyDizi
Copy link
Contributor Author

That's a good idea, I will start to work on it soon @mrT23

@KennyDizi
Copy link
Contributor Author

Start to implement it.

@KennyDizi
Copy link
Contributor Author

@mrT23 It's implemented via #1387. Please take a look at this PR.

@KennyDizi
Copy link
Contributor Author

Close this as completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants