Ruthless/improve metrics types 2 #474
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattieruth
force-pushed
the
ruthless/improve-metrics-types-2
branch
from
4ff35b6 to
0c20e9e
Compare
mattieruth
commented
Sep 18, 2024
| @@ -108,6 +126,7 @@ def __init__( | |||
|
|
|||
| # Metrics | |||
| self._metrics = FrameProcessorMetrics(name=self.name) | |||
| self._metrics_meta = {} | |||
There was a problem hiding this comment.
not to self: this is an artifact of a stale version. remove.
mattieruth
commented
Sep 18, 2024
src/pipecat/services/ai_services.py
Outdated
| @@ -11,6 +11,7 @@ | |||
| from abc import abstractmethod | |||
| from typing import AsyncGenerator, List, Optional, Tuple | |||
|
|
|||
| from attr import has | |||
There was a problem hiding this comment.
note to self: remove (not sure why this got added)
mattieruth
force-pushed
the
ruthless/improve-metrics-types-2
branch
6 times, most recently
from
346f2bb to
3a79564
Compare
mattieruth
commented
Sep 18, 2024
| @@ -212,7 +211,7 @@ async def process_frame(self, frame: Frame, direction: FrameDirection): | |||
| context = OpenAILLMContext.from_image_frame(frame) | |||
| elif isinstance(frame, LLMModelUpdateFrame): | |||
There was a problem hiding this comment.
theoretically, this can now move into LLMService and be another thing these processors don't have to remember to do
mattieruth
force-pushed
the
ruthless/improve-metrics-types-2
branch
5 times, most recently
from
2c815e7 to
e0b6629
Compare
mattieruth
force-pushed
the
ruthless/improve-metrics-types-2
branch
from
e0b6629 to
23af89c
Compare
aconchillo
reviewed
Sep 19, 2024
src/pipecat/services/ai_services.py
Outdated
| @@ -46,6 +47,11 @@ | |||
| class AIService(FrameProcessor): | |||
| def __init__(self, **kwargs): | |||
| super().__init__(**kwargs) | |||
| self.model_name: str = "" | |||
There was a problem hiding this comment.
could we make this private and a property instead?
@property
def model_name(self):
return self._model_name
There was a problem hiding this comment.
done. i think this is ready for final eyes :)
mattieruth
force-pushed
the
ruthless/get-tests-running
branch
from
5f0a3ed to
74005fb
Compare
mattieruth
force-pushed
the
ruthless/improve-metrics-types-2
branch
from
23af89c to
01cdb6e
Compare
mattieruth
force-pushed
the
ruthless/get-tests-running
branch
from
74005fb to
56a8429
Compare
mattieruth
force-pushed
the
ruthless/improve-metrics-types-2
branch
from
01cdb6e to
d84c4d7
Compare
mattieruth
force-pushed
the
ruthless/get-tests-running
branch
2 times, most recently
from
bc94a8a to
50b45ac
Compare
mattieruth
force-pushed
the
ruthless/improve-metrics-types-2
branch
from
d84c4d7 to
6bdf090
Compare
1. Fleshed out MetricsFrames and broke it into a proper set of types 2. Add model_name as a property to the AIService so that it can be automatically included in metrics and also remove that overhead from all the various services themselves Breaking change! Because of the types improvements, the MetricsFrame type has changed. Each frame will have a list of metrics simlilar to before except each item in the list will only contain one type of metric: "ttfb", "tokens", "characters", or "processing". Previously these fields would be in every entry but set to None if they didn't apply. While this changes internal handling of the MetricsFrame, it does NOT break the RTVI/daily messaging of metrics. That format remains the same. Also. Remember to use model_name for accessing a service's current model and set_model_name for setting it.
mattieruth
force-pushed
the
ruthless/improve-metrics-types-2
branch
from
6bdf090 to
a4edb3d
Compare
aconchillo
approved these changes
Sep 20, 2024
|
Nice! |
Thanks for all the help! |
cyrilS-dev
added a commit
to cyrilS-dev/pipecat
that referenced
this pull request
Sep 22, 2024
joachimchauvet
added a commit
to joachimchauvet/pipecat-livekit
that referenced
this pull request
Sep 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cleanup on aisle METRICS.
Note: See below, this is a breaking change
automatically included in metrics and also remove that
overhead from all the various services themselves
Breaking change!
Because of the types improvements, the MetricsFrame type has
changed. Each frame will have a list of metrics simlilar to before
except each item in the list will only contain one type of metric:
"ttfb", "tokens", "characters", or "processing". Previously these
fields would be in every entry but set to None if they didn't apply.
While this changes internal handling of the MetricsFrame, it does NOT
break the RTVI/daily messaging of metrics. That format remains the same.
Also. Remember to use model_name for accessing a service's current
model and set_model_name for setting it.