Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds version 0.1.1 of the roofline_analyzer CLI tool, introducing kernel ID numbering for plot annotations and full end-to-end Excel-based roofline analysis.
- Introduces an
IDcolumn for kernels and ties IDs into plot annotations. - Implements modular functions for reading, analyzing, plotting, and exporting Excel data.
- Updates project metadata (
pyproject.toml), sample config, and documentation for 0.1.1.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/custom_workflows/roofline_analyzer/src/xlsx.py | Excel data parsing, analysis column addition, and export functions |
| examples/custom_workflows/roofline_analyzer/src/roofline.py | Roofline model calculations and plot generation |
| examples/custom_workflows/roofline_analyzer/src/cli.py | Command-line interface for config handling and summary display |
| examples/custom_workflows/roofline_analyzer/pyproject.toml | Project metadata, version bump to 0.1.1, dependencies, and entry point |
| examples/custom_workflows/roofline_analyzer/mi300x_bfloat16.toml | Sample accelerator configuration for MI300X bfloat16 |
| examples/custom_workflows/roofline_analyzer/main.py | Main CLI command integrating all modules |
| examples/custom_workflows/roofline_analyzer/README.md | Installation, usage instructions, and output schema documentation |
| examples/custom_workflows/roofline_analyzer/.gitignore | Ignore patterns for build artifacts and generated files |
Comments suppressed due to low confidence (2)
examples/custom_workflows/roofline_analyzer/src/xlsx.py:1
- Key analysis and export functions lack accompanying unit tests; consider adding tests for
read_xlsx,add_analysis_columns, andexport_to_xlsxto verify behavior and guard against regressions.
import pandas as pd
examples/custom_workflows/roofline_analyzer/README.md:93
- [nitpick] Markdown bullets for
bound_distanceandbound_distance_pctaren't indented under the nested list for the analyzed sheet section; adjust indentation for consistent formatting.
* **bound_distance**: Distance to the nearest roofline
| def add_analysis_columns( | ||
| df: pd.DataFrame, | ||
| config: Dict[str, Any], | ||
| ai_ridge_mtf: float, |
There was a problem hiding this comment.
The parameter ai_ridge_mtf is never used in add_analysis_columns; consider removing it or using it as intended to keep the signature accurate.
| ai_ridge_mtf: float, |
| # Set config to use 'ID' as kernel_name_column for plotting | ||
| config['excel']['kernel_name_column'] = 'ID' |
There was a problem hiding this comment.
Mutating the passed config dict inside this function may lead to unexpected side effects; use a local variable for kernel_name_column instead of modifying the original config.
| # Set config to use 'ID' as kernel_name_column for plotting | |
| config['excel']['kernel_name_column'] = 'ID' | |
| # Use a local variable for 'ID' as kernel_name_column for plotting | |
| kernel_name_column = 'ID' |
I've created a python cli application that as an input takes the XLSX that is, to my knowledge. generated by `generate_perf_report.py` from #138 and when combined with a config-file that has e.g. MI300X bfloat16 values:  It will generate a naive roofline analysis for the GEMMs used in e.g. a client model:   and it will generate a new XLSX as an output, that has: 1. **bound_type**: what is the first limiting roofline (memory or compute), based on the "AI ridge point" 2. **reference_roofline**: what is used as the reference value for roofline distance calculation 3. **bound_distance**: what is the distance from kernel to the maf_roofline in question 4. **bound_distance_pct**: above in percentage  and AI ridge points for both the max achievable flops (maf) and max theoretical flops (mtf) i.e. the point that divides whether the kernel is memory or compute limited:  What is new in 0.1.1 compared to 0.1.0 is that the kernels will get a rolling numbering, which is also used to indicate which point on the plot belongs to which kernel:   For additional information, consult the extensive README.md
I've created a python cli application that
as an input takes the XLSX that is, to my knowledge. generated by
generate_perf_report.pyfrom#138
and when combined with a config-file that has e.g. MI300X bfloat16 values:

It will generate a naive roofline analysis for the GEMMs used in e.g. a client model:


and it will generate a new XLSX as an output, that has:
and AI ridge points for both the max achievable flops (maf) and max theoretical flops (mtf) i.e. the point that divides whether the kernel is memory or compute limited:
What is new in 0.1.1 compared to 0.1.0 is that the kernels will get a rolling numbering, which is also used to indicate which point on the plot belongs to which kernel:


For additional information, consult the extensive README.md