Skip to content
This repository was archived by the owner on Jun 24, 2024. It is now read-only.

feat: record inference stats #47

Merged
merged 2 commits into from
Mar 20, 2023
Merged

feat: record inference stats #47

merged 2 commits into from
Mar 20, 2023

Conversation

bcho
Copy link
Contributor

@bcho bcho commented Mar 19, 2023

This pull request added a simple struct InferenceStats for capturing the model evaluation stats. This struct can generate a similar output like llama.cpp . Closes #46

Example output:

feed_prompt_duration: 1615ms
prompt_tokens: 10
predict_duration: 6917ms
predict_tokens: 30
per_token_duration: 230.567ms

Copy link
Collaborator

@setzer22 setzer22 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for sharing! 😄 It was always bugging me that I couldn't see the times per token with this.

At first I thought maybe we needed to integrate this into the low-level functions feed_prompt and predict_next_token, but on second thought I prefer this version because it's simple, and if users are building a lower level loop using the other functions, they can do their own time measurements too, so there's no point in overengineering things.

Just have a very minor nitpick about newlines 🤔 but overall LGTM

fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(
f,
"feed_prompt_duration: {}ms, prompt_tokens: {}, predict_duration: {}ms, predict_tokens: {}, per_token_duration: {:.3}ms",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor nit: Wouldn't it make more sense to print these in separate lines?

What do others think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

np, updated, PTAL

Copy link
Collaborator

@setzer22 setzer22 left a comment

Choose a reason for hiding this comment

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

Thanks again! We can merge once CI passes :)

@setzer22 setzer22 merged commit d8ca18d into rustformers:main Mar 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reporting model stats
2 participants