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

Add possibility to select logging level. #2786

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
13 changes: 13 additions & 0 deletions src/promptflow-core/promptflow/_utils/logger_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,19 @@ def get_logger(name: str) -> logging.Logger:
service_logger = get_logger("execution.service")


def update_logger_levels(log_level: Optional[str] = None) -> None:
"""
Update the logger levels.

:param log_level: The new logging level. If it is None,
logging level will be taken from
using get_pf_logging_level.
:type log_level: Optional[str]
"""
for log in [flow_logger, bulk_logger, logger, service_logger]:
log.setLevel(log_level or get_pf_logging_level())


logger_contexts = []


Expand Down
10 changes: 7 additions & 3 deletions src/promptflow-core/promptflow/core/_serving/flow_invoker.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from promptflow._utils.dataclass_serializer import convert_eager_flow_output_to_dict
from promptflow._utils.flow_utils import dump_flow_result, is_executable_chat_flow
from promptflow._utils.logger_utils import LoggerFactory
from promptflow._utils.logger_utils import LoggerFactory, get_pf_logging_level
from promptflow._utils.multimedia_utils import MultimediaProcessor
from promptflow.core._connection import _Connection
from promptflow.core._connection_provider._connection_provider import ConnectionProvider
Expand Down Expand Up @@ -59,7 +59,10 @@ def __init__(
init_kwargs: dict = None,
**kwargs,
):
self.logger = kwargs.get("logger", LoggerFactory.get_logger("flowinvoker"))
self.logger = kwargs.get(
"logger",
LoggerFactory.get_logger("flowinvoker",
verbosity=kwargs.get('log_level') or get_pf_logging_level()))
self._init_kwargs = init_kwargs or {}
self.logger.debug(f"Init flow invoker with init kwargs: {self._init_kwargs}")
# TODO: avoid to use private attribute after we finalize the inheritance
Expand Down Expand Up @@ -123,7 +126,8 @@ def _init_connections(self, connection_provider):
connection_names=self.flow.get_connection_names(
environment_variables_overrides=os.environ,
),
provider=ConnectionProvider.init_from_provider_config(connection_provider, credential=self._credential),
provider=ConnectionProvider.init_from_provider_config(
connection_provider, credential=self._credential),
connections_to_ignore=connections_to_ignore,
# fetch connections with name override
connections_to_add=list(self.connections_name_overrides.values()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from functools import lru_cache
from os import PathLike
from pathlib import Path
from typing import Dict, Union
from typing import Dict, Optional, Union

from promptflow._sdk._configuration import Configuration
from promptflow._sdk._constants import NODES
Expand Down Expand Up @@ -37,19 +37,20 @@ def __init__(self, flow_path: PathLike):

@classmethod
@lru_cache
def resolve(cls, flow: Flow) -> "FlowInvoker":
def resolve(cls, flow: Flow, log_level: Optional[int] = None) -> "FlowInvoker":
"""Resolve flow to flow invoker."""
resolver = cls(flow_path=flow.path)
resolver._resolve(flow_context=flow.context)
return resolver._create_invoker(flow_context=flow.context)
return resolver._create_invoker(flow_context=flow.context, log_level=log_level)

@classmethod
@lru_cache
def resolve_async_invoker(cls, flow: Flow) -> "AsyncFlowInvoker":
def resolve_async_invoker(cls, flow: Flow, log_level: Optional[int] = None) -> "AsyncFlowInvoker":
"""Resolve flow to flow invoker."""
resolver = cls(flow_path=flow.path)
resolver._resolve(flow_context=flow.context)
return resolver._create_invoker(flow_context=flow.context, is_async_call=True)
return resolver._create_invoker(flow_context=flow.context, is_async_call=True,
log_level=log_level)

def _resolve(self, flow_context: FlowContext):
"""Resolve flow context."""
Expand Down Expand Up @@ -113,7 +114,8 @@ def _resolve_connection_objs(self, flow_context: FlowContext):
return connections

def _create_invoker(
self, flow_context: FlowContext, is_async_call=False
self, flow_context: FlowContext, is_async_call=False,
log_level: Optional[int] = None
) -> Union["FlowInvoker", "AsyncFlowInvoker"]:
from promptflow.core._serving.flow_invoker import AsyncFlowInvoker, FlowInvoker

Expand All @@ -132,11 +134,13 @@ def _create_invoker(
flow=resolved_flow,
connections=connections,
streaming=flow_context.streaming,
log_level=log_level,
)
else:
return FlowInvoker(
flow=resolved_flow,
connections=connections,
streaming=flow_context.streaming,
connection_provider=Configuration.get_instance().get_connection_provider(),
log_level=log_level,
)
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from promptflow._constants import DEFAULT_ENCODING, FLOW_FILE_SUFFIX
from promptflow._sdk.entities._validation import SchemaValidatableMixin
from promptflow._utils.flow_utils import is_flex_flow, is_prompty_flow, resolve_flow_path
from promptflow._utils.logger_utils import update_logger_levels
from promptflow._utils.yaml_utils import load_yaml_string
from promptflow.core._flow import AbstractFlowBase
from promptflow.exceptions import UserErrorException
Expand Down Expand Up @@ -145,6 +146,7 @@ def __init__(
**kwargs,
):
self.variant = kwargs.pop("variant", None) or {}
self._log_level = kwargs.pop("log_level", None)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this setting should not be in Flow obj as it's changes static loggers which is not part of flow

super().__init__(data=dag, code=code, path=path, **kwargs)

@property
Expand Down Expand Up @@ -236,14 +238,16 @@ def __call__(self, *args, **kwargs):
if args:
raise UserErrorException("Flow can only be called with keyword arguments.")

if self._log_level:
update_logger_levels(self._log_level)
result = self.invoke(inputs=kwargs)
return result.output

def invoke(self, inputs: dict) -> "LineResult":
"""Invoke a flow and get a LineResult object."""
from promptflow._sdk.entities._flows._flow_context_resolver import FlowContextResolver

invoker = FlowContextResolver.resolve(flow=self)
invoker = FlowContextResolver.resolve(flow=self, log_level=self._log_level)
result = invoker._invoke(
data=inputs,
)
Expand Down
2 changes: 1 addition & 1 deletion src/promptflow-devkit/tests/sdk_cli_test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import pytest
from _constants import CONNECTION_FILE, PROMPTFLOW_ROOT
from mock import mock
from unittest import mock
from pytest_mock import MockerFixture
from sqlalchemy import create_engine

Expand Down
34 changes: 19 additions & 15 deletions src/promptflow-evals/promptflow/evals/evaluate/_evaluate.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# ---------------------------------------------------------
import inspect
from types import FunctionType
from typing import Callable, Dict, Optional
from typing import Callable, Dict, Optional, Union, cast

import pandas as pd

Expand Down Expand Up @@ -103,23 +103,27 @@ def evaluate(
code_client = CodeClient()

evaluator_info = {}

for evaluator_name, evaluator in evaluators.items():
if isinstance(evaluator, FunctionType):
evaluator_info.update({evaluator_name: {"client": pf_client, "evaluator": evaluator}})
else:
evaluator_info.update({evaluator_name: {"client": code_client, "evaluator": evaluator}})

evaluator_info[evaluator_name]["run"] = evaluator_info[evaluator_name]["client"].run(
flow=evaluator,
column_mapping=evaluator_config.get(evaluator_name, evaluator_config.get("default", None)),
data=data,
stream=True,
)
if evaluator_config is None:
evaluator_config = {}

if evaluators:
for evaluator_name, evaluator in evaluators.items():
if isinstance(evaluator, FunctionType):
evaluator_info.update({evaluator_name: {"client": pf_client, "evaluator": evaluator}})
else:
evaluator_info.update({evaluator_name: {"client": code_client, "evaluator": evaluator}})

evaluator_info[evaluator_name]["run"] = evaluator_info[evaluator_name]["client"].run(
flow=evaluator,
column_mapping=evaluator_config.get(evaluator_name, evaluator_config.get("default", None)),
data=data,
stream=True,
)

evaluators_result_df = None
for evaluator_name, evaluator_info in evaluator_info.items():
evaluator_result_df = evaluator_info["client"].get_details(evaluator_info["run"], all_results=True)
evaluator_result_df = cast(
Union[PFClient, CodeClient], evaluator_info["client"]).get_details(evaluator_info["run"], all_results=True)

# drop input columns
evaluator_result_df = evaluator_result_df.drop(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from promptflow.connections import AzureOpenAIConnection
from promptflow.evals.evaluators import GroundednessEvaluator, RelevanceEvaluator, CoherenceEvaluator, FluencyEvaluator
from typing import List, Dict
from typing import Any, Dict, List
from concurrent.futures import ThreadPoolExecutor, as_completed
import json
import logging
Expand Down Expand Up @@ -170,8 +170,8 @@ def _evaluate_turn(self, turn_num, questions, answers, contexts, evaluator):
return {}

def _aggregate_results(self, per_turn_results: List[Dict]):
scores = {}
reasons = {}
scores: Dict[str, Any] = {}
reasons: Dict[str, Any] = {}

for turn in per_turn_results:
for metric, value in turn.items():
Expand All @@ -184,7 +184,7 @@ def _aggregate_results(self, per_turn_results: List[Dict]):
scores[metric] = []
scores[metric].append(value)

aggregated = {}
aggregated: Dict[str, Any] = {}
evaluation_per_turn = {}

for metric, values in scores.items():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
import numpy as np
import re

from typing import Optional


@tool
def parse_score(llm_output: str = None):
def parse_score(llm_output: Optional[str] = None):
score = np.nan
if llm_output:
match = re.search(r'\d', llm_output)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from promptflow.core import tool
import time
from constants import RAIService, Tasks
from typing import List
from typing import Any, Dict, List
import numpy as np
import re
from utils import get_harm_severity_level
Expand Down Expand Up @@ -68,7 +68,7 @@ def fetch_result(operation_id: str, rai_svc_url: str, credential: TokenCredentia
time.sleep(sleep_time)


def parse_response(batch_response: List[dict], metric_name: str) -> List[List[dict]]:
def parse_response(batch_response: List[dict], metric_name: str) -> Dict[str, Any]:
# Fix the metric name if it's "hate_fairness"
# Eventually we will remove this fix once the RAI service is updated
key = metric_name
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import constants
import numpy as np

from typing import Union

def get_harm_severity_level(harm_score: int) -> str:

def get_harm_severity_level(harm_score: int) -> Union[str, float]:
HARM_SEVERITY_LEVEL_MAPPING = {constants.HarmSeverityLevel.Safe: [0, 1],
constants.HarmSeverityLevel.Low: [2, 3],
constants.HarmSeverityLevel.Medium: [4, 5],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@


class HateUnfairnessEvaluator:
def __init__(self, project_scope: dict, credential: Optional[TokenCredential] = None):
def __init__(self, project_scope: dict, credential: Optional[TokenCredential] = None,
log_level: Optional[int] = None):
"""
Initialize an evaluator for hate unfairness score.

Expand All @@ -15,6 +16,8 @@ def __init__(self, project_scope: dict, credential: Optional[TokenCredential] =
:type project_scope: dict
:param credential: The credential for connecting to Azure AI project.
:type credential: TokenCredential
:param log_level: The logging level.
:type log_level: Optional[int]

**Usage**

Expand All @@ -30,7 +33,7 @@ def __init__(self, project_scope: dict, credential: Optional[TokenCredential] =
# Load the flow as function
current_dir = Path(__file__).resolve().parent
flow_dir = current_dir / "flow"
self._flow = load_flow(source=flow_dir)
self._flow = load_flow(source=flow_dir, log_level=log_level)
Copy link
Contributor

Choose a reason for hiding this comment

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

could we directly set the logger here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or just give user some doc that they can change logging behavior by setting some env variable.

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 main problem here is that all the log records, the task are not coming from flow itself, it is from invoker/executor infrastructure.
We are using two mechanisms of getting loggers. If user will set the PF_LOGGING_LEVEL, the flow executor logs will ignore it, because the environment variable will have to be set before importing promptflow, as the logger is initialized during import (see the logger_utils.py).

flow_logger = get_logger("execution.flow")

Another problem is that we initialize the "flowinvoker" logger on flow invoker level, using different mechanism, which will respect PF_LOGGING_LEVEL (the code is given before this PR):

self.logger = kwargs.get("logger", LoggerFactory.get_logger("flowinvoker"))

So that the logger created/set on the flow level will not have the effect. All


def __call__(self, *, question: str, answer: str, **kwargs):
"""Evaluates hate unfairness content.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@


class SelfHarmEvaluator:
def __init__(self, project_scope: dict, credential: Optional[TokenCredential] = None):
def __init__(self, project_scope: dict, credential: Optional[TokenCredential] = None,
log_level: Optional[int] = None):
"""
Initialize an evaluator for self harm score.

Expand All @@ -15,6 +16,8 @@ def __init__(self, project_scope: dict, credential: Optional[TokenCredential] =
:type project_scope: dict
:param credential: The credential for connecting to Azure AI project.
:type credential: TokenCredential
:param log_level: The logging level.
:type log_level: Optional[int]

**Usage**

Expand All @@ -30,7 +33,7 @@ def __init__(self, project_scope: dict, credential: Optional[TokenCredential] =
# Load the flow as function
current_dir = Path(__file__).resolve().parent
flow_dir = current_dir / "flow"
self._flow = load_flow(source=flow_dir)
self._flow = load_flow(source=flow_dir, log_level=log_level)

def __call__(self, *, question: str, answer: str, **kwargs):
"""Evaluates self harm content.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@


class SexualEvaluator:
def __init__(self, project_scope: dict, credential: Optional[TokenCredential] = None):
def __init__(self, project_scope: dict, credential: Optional[TokenCredential] = None,
log_level: Optional[int] = None):
"""
Initialize an evaluator for sexual score.

Expand All @@ -15,6 +16,8 @@ def __init__(self, project_scope: dict, credential: Optional[TokenCredential] =
:type project_scope: dict
:param credential: The credential for connecting to Azure AI project.
:type credential: TokenCredential
:param log_level: The logging level.
:type log_level: Optional[int]

**Usage**

Expand All @@ -30,7 +33,7 @@ def __init__(self, project_scope: dict, credential: Optional[TokenCredential] =
# Load the flow as function
current_dir = Path(__file__).resolve().parent
flow_dir = current_dir / "flow"
self._flow = load_flow(source=flow_dir)
self._flow = load_flow(source=flow_dir, log_level=log_level)

def __call__(self, *, question: str, answer: str, **kwargs):
"""Evaluates sexual content.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@


class ViolenceEvaluator:
def __init__(self, project_scope: dict, credential: Optional[TokenCredential] = None):
def __init__(self, project_scope: dict, credential: Optional[TokenCredential] = None,
log_level: Optional[int] = None):
"""
Initialize an evaluator for violence score.

Expand All @@ -15,6 +16,8 @@ def __init__(self, project_scope: dict, credential: Optional[TokenCredential] =
:type project_scope: dict
:param credential: The credential for connecting to Azure AI project.
:type credential: TokenCredential
:param log_level: The logging level.
:type log_level: Optional[int]

**Usage**

Expand All @@ -30,7 +33,7 @@ def __init__(self, project_scope: dict, credential: Optional[TokenCredential] =
# Load the flow as function
current_dir = Path(__file__).resolve().parent
flow_dir = current_dir / "flow"
self._flow = load_flow(source=flow_dir)
self._flow = load_flow(source=flow_dir, log_level=log_level)

def __call__(self, *, question: str, answer: str, **kwargs):
"""Evaluates violence content.
Expand Down
Loading
Loading