Skip to content

compare llama-bench: add option to plot #14169

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

Merged
merged 7 commits into from
Jun 14, 2025

Conversation

am17an
Copy link
Collaborator

@am17an am17an commented Jun 13, 2025

Creates graph with varying one variable (default: n_depth) and keeping others constant, added --plot (for plot location) and --plot_x (for x-axis) in the script

example graph
image

Copy link
Collaborator

@JohannesGaessler JohannesGaessler left a comment

Choose a reason for hiding this comment

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

Thank you, this would be useful for my work. For more general use consider also adding support for plotting the performance of only a single commit. Non-developer users are frequently more interested in determining the performance of their setup rather than the performance of the software so they would be I think more interested in running llama-bench for only a single commit and plotting the performance of that. In practical terms the interface change that I envision would be to make --compare optional. The opposite case where you may want to compare more than two commits could also be interesting. (These things should be in separate PRs.)

@JohannesGaessler
Copy link
Collaborator

Regarding capitalization: I'm going by the standards that are common in my line of work unrelated to llama.cpp (high energy physics) where the guideline is not to capitalize every word in plot titles and axis names. Because the PRETTY_NAMES are capitalized this is going to be inconsistent with a join on ", " but this is I think a minor issue and would not block a merge.

@CISC
Copy link
Collaborator

CISC commented Jun 13, 2025

Thank you, this would be useful for my work. For more general use consider also adding support for plotting the performance of only a single commit. Non-developer users are frequently more interested in determining the performance of their setup rather than the performance of the software so they would be I think more interested in running llama-bench for only a single commit and plotting the performance of that. In practical terms the interface change that I envision would be to make --compare optional. The opposite case where you may want to compare more than two commits could also be interesting. (These things should be in separate PRs.)

Just FYI, I haven't forgotten about adding Mermaid support, but I was working on comparing across columns first (ie, comparing two different models or options) and got stuck in SQL hell so it got stashed and put on the back burner. :(

@JohannesGaessler
Copy link
Collaborator

Sorry, but I'm not going to turn down this PR when it does what I need with the tools that I'm used to.

@CISC
Copy link
Collaborator

CISC commented Jun 13, 2025

Sorry, but I'm not going to turn down this PR when it does what I need with the tools that I'm used to.

Oh, no, I wasn't suggesting that, this PR looks great!

@github-actions github-actions bot added script Script related python python script changes labels Jun 13, 2025
@am17an am17an force-pushed the add_plot_compare_llama_bench branch from 10042de to 567b2cb Compare June 13, 2025 13:27
@am17an am17an force-pushed the add_plot_compare_llama_bench branch from 567b2cb to deeaecf Compare June 13, 2025 13:28
@JohannesGaessler
Copy link
Collaborator

Test plot I made for a currently open PR:

plot

I manually edited the code to get a log scale.

Copy link
Collaborator

@JohannesGaessler JohannesGaessler left a comment

Choose a reason for hiding this comment

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

If there's nothing else from your side I'll merge.

@JohannesGaessler JohannesGaessler merged commit 2e42be4 into ggml-org:master Jun 14, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python python script changes script Script related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants