Repository navigation
feat(rocprofiler-compute) Data fix and UX improvement for roofline - #12190
cfallows-amd merged 46 commits into
Conversation
|
@cfallows-amd and @xuchen-amd could you please take the ownership of this PR and keep it rebased and up-to-date and implement review findings after talking to @svolkov-amd - please also finish the testing, thanks! |
95d6a38 to
fb4414e
Compare
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
Empty commit to open the sprint integration PR against develop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Empty commit to open the sprint integration PR against develop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This reverts commit 2c3e908.
|
Ctest passing on MI350X and MI455X. Ran a 10 and 100 experiment benchmark cycle to do a comparison: |
There was a problem hiding this comment.
🟡 Changes recommended
Dispatch percentages use a filtered denominator, and the truncated kernel controls do not expose full names accessibly.
5 open findings
Expose full action name through aria-label · New Use app-wide dispatch count for percentage denominator · New Clarify fallback behavior for compute-bound and unknown levels · New Add deterministic tests for maximum benchmark aggregation · New Fix missing whitespace and inaccurate limiter description · New
What changed in this PR
Improves roofline accuracy and usability through corrected benchmark calculations, richer tooltips, and limiter-focused plotting.
Changes:
- Adds limiting-peak axis mode, kernel-name truncation, and detailed hover data.
- Uses peak benchmark samples and corrects gfx950 FP16 MFMA.
- Expands default precisions and updates tests/documentation.
| File | Description |
|---|---|
tests/unit/utils/test_roofline_calc.py |
Updates kernel-stat field expectations. |
tests/unit/roofline/test_roofline_main.py |
Tests limiter, tooltip, precision, and truncation behavior. |
src/utils/roofline_calc.py |
Adds application and renamed kernel timing data. |
src/roofline/roofline_main.py |
Implements limiter selection and richer kernel models. |
src/roofline/roofline_html.py |
Embeds limiter and tooltip assets. |
src/roofline/roofline_hover.py |
Restructures hover content and formatting. |
src/roofline/benchmark/gfx9/benchmark_gfx950.py |
Corrects FP16 MFMA intrinsic accounting. |
src/roofline/benchmark/benchmark_base.py |
Records peak benchmark samples and simplifies CSV output. |
src/roofline/assets/roofline_plot.js |
Adds limiting-axis and kernel-tooltip behavior. |
src/roofline/assets/roofline_kernel_list_tooltip.js |
Implements full-name tooltips. |
src/roofline/assets/roofline_kernel_list_tooltip.css |
Styles kernel-name tooltips. |
docs/how-to/analyze/cli.rst |
Documents the limiting-axis mode. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…e full kernel name when truncated, so screen readers announce the complete name. roofline_calc.py, roofline_main.py: totalAppDispatches added to PlotPoints, populated from top_df["Count"].sum() (the full unfiltered table). roofline_main.py now reads it from ai_data instead of summing the filtered counts list — fixes the case where a selected kernel could show 100% dispatch share. roofline_html.py, cli.rst: Missing space between adjacent string literals fixe, Inaccurate "bottleneck points" description replaced with accurate fallback wording in both the in-HTML tooltip and the docs. Signed-off-by: Carrie Fallows <Carrie.Fallows@amd.com>
Create CHANGELOG entries. Signed-off-by: Carrie Fallows <Carrie.Fallows@amd.com>
feizheng10
left a comment
There was a problem hiding this comment.
Two problems in how a kernel's limiter is chosen and how Limited by picks the point to draw.
|
Using fake data to demonstrate the two problems above. The intensities, bandwidths, and peaks are invented. They follow the limiter rules on this pull request. This is not a profiled workload. 1. A kernel above the FP32 peaks is labeled LDS.
The same kernel at 120,000 GFLOP/s has no roof left above it, so the hover is Unknown. The drawn FP8 ceiling is still 160,000. 2. A compute-bound kernel is drawn on LDS. Second fake machine: LDS 50,000, L1 12,000, L2 2,400, HBM 500 GB/s, FP32 VALU 10,000, FP32 MFMA 20,000 GFLOP/s.
Working on a fix on branch |
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Pushed a commit removing the unused |
…CHANGELOG Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Pushed a CHANGELOG entry for the higher gfx950 FP16/BF16 MFMA peaks, since that's what users see from the instruction change. |
- Indent the Limited by docs line so the AI axis bullet list renders - Use max(samples) directly instead of calc_stats - Move roofline_main test imports to module level - Replace the nested ternary in exportViewSubtitle with peakLabel Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Pushed a few small fixes:
|
…candidate Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Pushed a fix so the limiter's compute candidates come from every datatype drawn on the page, not just |
|
@feizheng10 thanks for the detailed walkthrough, the exceeded-roof cases are a fair concern. Handling kernels above a measured roof is out of scope for this PR, so let's take it up in a follow-up. |
|
Fei's comments are valid and we will continue to correct the limiter on the plot through his new branch. Team has agreed to push this in now and create followup PRs. |
8feab02
into
rocprofiler-compute-develop
- Route the roofline kernel list tooltip assets from #12190 through build_document. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…12190) ## Motivation Rocprofiler-compute had incorrect data and was hard to use. Therefore, while doing an analysis in the roofline, I've fixed some of these issues. ## Technical Details Changes are: - Kernel names are limited in size to 200 symbols. One of the side effects of the long kernel name was that for extremely long kernel names, their preview in tooltip glithched. - Tooltip which shows a full kernel name is added when you hover on a kernel in the kernel list on the right. - Tooltip for kernel on the plot is changed to structure the data better, so it's easier to inspect it. - New "AI axis" mode is added which shows a kernel point which is a bottleneck (ex. if L1 is a bottleneck, the L1 point is shown. If kernel is compute bound then the left-most point is shown). This mode is made default. - Default state of HTML is updated to always show FP32, FP16, FP8 rooflines when HTML is opened. - Roofline benchmark is updated: - FP16 and BF16 MFMA intrinsics updated. - Max is taken for roofline value instead of average. ## Issue Tracking AIPROFCOMP-853 ## Test Plan Manual validation for various pytorch bmm operators. Unit tests. ## Test Result Kernels show a proper limiter now, metrics in tooltip make sense, unit tests pass. ## Submission Checklist - [x ] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. [AIPROFCOMP-232]: https://amd-hub.atlassian.net/browse/AIPROFCOMP-232?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Signed-off-by: Carrie Fallows <Carrie.Fallows@amd.com> Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Carrie Fallows <Carrie.Fallows@amd.com>
…12190) ## Motivation Rocprofiler-compute had incorrect data and was hard to use. Therefore, while doing an analysis in the roofline, I've fixed some of these issues. ## Technical Details Changes are: - Kernel names are limited in size to 200 symbols. One of the side effects of the long kernel name was that for extremely long kernel names, their preview in tooltip glithched. - Tooltip which shows a full kernel name is added when you hover on a kernel in the kernel list on the right. - Tooltip for kernel on the plot is changed to structure the data better, so it's easier to inspect it. - New "AI axis" mode is added which shows a kernel point which is a bottleneck (ex. if L1 is a bottleneck, the L1 point is shown. If kernel is compute bound then the left-most point is shown). This mode is made default. - Default state of HTML is updated to always show FP32, FP16, FP8 rooflines when HTML is opened. - Roofline benchmark is updated: - FP16 and BF16 MFMA intrinsics updated. - Max is taken for roofline value instead of average. ## Issue Tracking AIPROFCOMP-853 ## Test Plan Manual validation for various pytorch bmm operators. Unit tests. ## Test Result Kernels show a proper limiter now, metrics in tooltip make sense, unit tests pass. ## Submission Checklist - [x ] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. [AIPROFCOMP-232]: https://amd-hub.atlassian.net/browse/AIPROFCOMP-232?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Signed-off-by: Carrie Fallows <Carrie.Fallows@amd.com> Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Carrie Fallows <Carrie.Fallows@amd.com>
…12190) ## Motivation Rocprofiler-compute had incorrect data and was hard to use. Therefore, while doing an analysis in the roofline, I've fixed some of these issues. ## Technical Details Changes are: - Kernel names are limited in size to 200 symbols. One of the side effects of the long kernel name was that for extremely long kernel names, their preview in tooltip glithched. - Tooltip which shows a full kernel name is added when you hover on a kernel in the kernel list on the right. - Tooltip for kernel on the plot is changed to structure the data better, so it's easier to inspect it. - New "AI axis" mode is added which shows a kernel point which is a bottleneck (ex. if L1 is a bottleneck, the L1 point is shown. If kernel is compute bound then the left-most point is shown). This mode is made default. - Default state of HTML is updated to always show FP32, FP16, FP8 rooflines when HTML is opened. - Roofline benchmark is updated: - FP16 and BF16 MFMA intrinsics updated. - Max is taken for roofline value instead of average. ## Issue Tracking AIPROFCOMP-853 ## Test Plan Manual validation for various pytorch bmm operators. Unit tests. ## Test Result Kernels show a proper limiter now, metrics in tooltip make sense, unit tests pass. ## Submission Checklist - [x ] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. [AIPROFCOMP-232]: https://amd-hub.atlassian.net/browse/AIPROFCOMP-232?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Signed-off-by: Carrie Fallows <Carrie.Fallows@amd.com> Co-authored-by: Vignesh Edithal <Vignesh.Edithal@amd.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Carrie Fallows <Carrie.Fallows@amd.com>



Motivation
Rocprofiler-compute had incorrect data and was hard to use. Therefore, while doing an analysis in the roofline, I've fixed some of these issues.
Technical Details
Changes are:
Issue Tracking
AIPROFCOMP-853
Test Plan
Manual validation for various pytorch bmm operators.
Unit tests.
Test Result
Kernels show a proper limiter now, metrics in tooltip make sense, unit tests pass.
Submission Checklist