-
Notifications
You must be signed in to change notification settings - Fork 7
feat: DEP for AIPerf Visualization Feature and Plot Command #47
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
base: main
Are you sure you want to change the base?
Conversation
lkomali
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job on detailing the project. Liked the idea of interactive plots.
I have some comments, open to discuss.
In general, my opinion is that analyze and plot should be decoupled.
profilewill benchmark and generate results. Users might want to visualize individual runs as well as compare across runs.analyzewill sweep across different values for a parameter and provide results. Users runninganalyzemay or may not want to visualize their results.plotshould be able to visualize results coming both the above subcommands. Users should be able to visualize given a set of result files irrespective of the step before ranprofileoranalyzecommand.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This is a great proposal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm halfway through it, but really good design.
Have a couple of small comments.
Will take another pass for the other half.
lkomali
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Great work @ilana-n
| ### REQ 5 Essential Plot Types | ||
|
|
||
| The system **MUST** provide the following essential visualizations: | ||
| - **Comparison plots**: Pareto curves, metric vs metric (ex: latency vs throughput), metric vs parameter (ex: output token throughput per user vs concurrency), distribution comparisons |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is room for a three dimensional plot for the pareto curves and metric comparisons since these metrics can change for the different ISL/OSL counts. Being able to make effective 3D visualizations would be pushing the frontier here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would be great, thanks for the feedback! Will add this - it will likely will be additional work after basic functionality is finished.
|
|
||
| # Future Enhancements | ||
|
|
||
| ## Connector with Dynamo Metrics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This work is on the backlog for AIPerf and being tracked here:
https://linear.app/nvidia/issue/AIP-329/integrate-server-side-metrics-with-aiperf-runs
https://linear.app/nvidia/issue/AIP-274/add-kv-cache-performance-reporting
Add AIPerf Visualization Feature Design Plan
Proposes
aiperf plotcommand to generate visualizations from profiling results.Key features:
Includes:
This addresses the common need to visualize performance metrics, compare runs, and analyze trade-offs without external tools.