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

function to emit metrics #1649

Merged
merged 2 commits into from
May 17, 2024
Merged

function to emit metrics #1649

merged 2 commits into from
May 17, 2024

Conversation

technillogue
Copy link
Contributor

@technillogue technillogue commented May 6, 2024

we need the model code to emit accurate input token counts using the correct prompt formatting and tokenizer for the model in question. after some internal discussion from cog import emit_metric seems similar to the best approach for this. presently these metrics are ignored for non-official models and shouldn't really matter anywhere else, so it shouldn't be a concern with billing. this particular approach is janky but should unblock us for now.

usage:

     async def predict(self, ...) -> ...:
        ...
        formatted_prompt = prompt_template.format(prompt=prompt, system_prompt=system_prompt)
        input_token_count = len(self.tokenizer(formatted_prompt )["input_ids"])
        cog.emit_metric(name="input_token_count", value=input_token_count)
        ...

@technillogue technillogue requested review from bfirsh and a team May 6, 2024 22:48
@technillogue technillogue force-pushed the syl/more-refactor branch 5 times, most recently from 53a5318 to 84cece7 Compare May 14, 2024 05:31
@technillogue technillogue force-pushed the syl/more-refactor branch 5 times, most recently from 7098fde to 40bdb54 Compare May 16, 2024 20:18
Base automatically changed from syl/more-refactor to async May 16, 2024 21:08
@technillogue technillogue force-pushed the syl/emit-metric branch 2 times, most recently from 5a0d81d to 56aa9d8 Compare May 17, 2024 00:13
@technillogue
Copy link
Contributor Author

TODO new doc file

Copy link
Member

@zeke zeke left a comment

Choose a reason for hiding this comment

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

I can't speak to the technical implementation of this, but I think it needs docs.

I would suggest creating a docs/metrics.md file to document why these exists, and the API for sending metrics. Maybe also some recommendations like "Use snake case for metric names" or "Put an x_ prefix on your custom metrics names" or similar.

docs/metrics.md Outdated
@@ -0,0 +1,14 @@
# Metrics

Prediction objects have a `metrics` field. This normally includes `predict_time` and `total_time`. Official language models have metrics like `input_token_count`, `output_token_count`, `tokens_per_second`, and `time_to_first_token`. Currently, custom metrics from Cog are ignored when running on Replicate. Official Replicate-published models are the only exception to this. When running outside of Cog, you can emit custom metrics like this:
Copy link
Member

Choose a reason for hiding this comment

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

What does "When running outside of Cog" mean? Should this be "outside of Replicate"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed; we'll also have another chance to review this when merging into main

Signed-off-by: technillogue <technillogue@gmail.com>
Signed-off-by: technillogue <technillogue@gmail.com>
@technillogue technillogue merged commit b35d1f7 into async May 17, 2024
10 checks passed
@technillogue technillogue deleted the syl/emit-metric branch May 17, 2024 21:45
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

Successfully merging this pull request may close these issues.

None yet

3 participants