A Few Ideas to Enhance the optimize_llm Tool

[KatoStevenMubiru]

After looking at the optimize_llm tool in optimize_llm_tool.py and had a few thoughts on how we could make it even better. Nothing major, just some small tweaks to potentially enhance clarity and usability. 😊

Docstring Clarity:
The current docstring is good, but I was wondering if we could expand it a bit to give more details about the config options. It would be really helpful to have clear explanations for each parameter (like "quality," "cost," etc.), including what kind of values they expect and what those values represent.

For example, it would be great to specify the units for "time_to_first_token" (seconds?) and what the different quality levels ("1", "2", "3") actually mean.

Maybe something like this?

def optimize_llm(connection: Unify, config: Optional[dict], input_text: Union[str, Sequence]) -> Union[dict, str]:
    """
    Selects the optimal model for a step of a flow based on specified criteria.

    :param connection: Unify client to use.
    :param config: A dictionary specifying the optimization requirements. This can include:
                   - **quality:** A string representing the desired quality level (e.g., "1", "2", "3"). Higher values indicate higher quality.
                   - **cost:** A string representing the maximum cost per token (e.g., "4.65e-03").
                   - **time_to_first_token:** A string representing the maximum acceptable time to first token in seconds (e.g., "2.08e-05").
                   - **inter_token_latency:** A string representing the maximum acceptable inter-token latency in seconds (e.g., "2.07e-03").
                   - **endpoint:**  An optional string or list of strings specifying preferred endpoints (e.g., "gpt-3.5-turbo@openai").
                   - **model:** An optional string or list of strings specifying preferred models (e.g., "gpt-3.5-turbo").
                   - **provider:** An optional string or list of strings specifying preferred providers (e.g., "openai"). 
    :param input_text: The input text for the LLM. Can be a string or a sequence of strings. 
    :return: A dictionary containing the optimal endpoint and the LLM response, or a string if an error occurs.
    """
    # ... existing code ...

Variable Names:
Just a small thought—would it be possible to use more descriptive variable names for those single-letter variables (q, c, t, i) in the router string? Something like quality_weight, cost_weight, etc. might make the code a bit easier to follow. 😊

Input Validation:
It might be a good idea to add some checks to make sure the values provided in the config dictionary are valid. This could help prevent unexpected issues or errors if someone accidentally enters the wrong type of value.

I think these small changes could make the optimize_llm tool even clearer and more user-friendly. What do you think? 🤔

[Kacper-W-Kozdon]

Will do! Great suggestion. I'll also try to maybe add a separate tool to set up the config files, so it's displayed in the UI as well.

[KatoStevenMubiru]

Awesome, thanks Kacper! 👍 The idea of a separate tool for setting up the config files sounds great.

[Kacper-W-Kozdon]

Awesome, thanks Kacper! 👍 The idea of a separate tool for setting up the config files sounds great.

Descriptive names are already there, only the tool to set up the configs needs to be added.

    quality: str = config.get("quality", "1")
    cost: str = config.get("cost", "4.65e-03")
    time_to_first_token: str = config.get("time_to_first_token", "2.08e-05")
    inter_token_latency: str = config.get("inter_token_latency", "2.07e-03")
    endpoint: Union[list[str], str] = config.get("endpoint", None)
    model: Union[list[str], str] = config.get("model", None)
    provider: Union[list[str], str] = config.get("provider", None)

    router: str = f"router@q:{quality}|c:{cost}|t:{time_to_first_token}|i:{inter_token_latency}"

[KatoStevenMubiru]

Thanks for updating the docstring! For the router string, I was referring to those variables within the string itself (q:{quality}, c:{cost}, etc.). It would be great to replace those with more descriptive names like quality_weight, cost_weight, etc.

Also, have you had a chance to consider adding input validation to the config dictionary?

I've included a code example below to illustrate these suggestions, but I won't be pushing these changes to the main branch yet.

from typing import Optional, Sequence, Union

from unify import Unify

from promptflow.core import tool

@tool
def optimize_llm(connection: Unify, config: Optional[dict], input_text: Union[str, Sequence] = " ") -> Union[dict, str]:
    """
    Selects the optimal model for a step of a flow.

    # ... (Rest of docstring) ...
    """
    assert isinstance(connection.get_credit_balance(), (str, float, int))
    
    if not isinstance(config, dict):
        config = {}
    
    # Input validation
    def validate_weight(value, name):
        try:
            float(value)
        except ValueError:
            raise ValueError(f"{name} must be a valid float string")

    quality_weight: str = config.get("quality_weight", "1")
    cost_weight: str = config.get("cost_weight", "4.65e-03")
    # ... (Rest of the function) ...

[KatoStevenMubiru]

Let me know what you think about these ideas! I'm happy to discuss them further.

Thanks again!

[Kacper-W-Kozdon]

Thanks for updating the docstring! For the router string, I was referring to those variables within the string itself (q:{quality}, c:{cost}, etc.). It would be great to replace those with more descriptive names like quality_weight, cost_weight, etc.

Also, have you had a chance to consider adding input validation to the config dictionary?

I've included a code example below to illustrate these suggestions, but I won't be pushing these changes to the main branch yet.

from typing import Optional, Sequence, Union

from unify import Unify

from promptflow.core import tool

@tool
def optimize_llm(connection: Unify, config: Optional[dict], input_text: Union[str, Sequence] = " ") -> Union[dict, str]:
    """
    Selects the optimal model for a step of a flow.

    # ... (Rest of docstring) ...
    """
    assert isinstance(connection.get_credit_balance(), (str, float, int))
    
    if not isinstance(config, dict):
        config = {}
    
    # Input validation
    def validate_weight(value, name):
        try:
            float(value)
        except ValueError:
            raise ValueError(f"{name} must be a valid float string")

    quality_weight: str = config.get("quality_weight", "1")
    cost_weight: str = config.get("cost_weight", "4.65e-03")
    # ... (Rest of the function) ...

Ok, I'll use that as an opportunity to separate config preparation as an additional tool then.

[KatoStevenMubiru]

Alright then