Conversation
abchoudh-amd
force-pushed
the
list_blocks_bug
branch
from
8ded1e1 to
d46c301
Compare
There was a problem hiding this comment.
Pull request overview
Fixes --list-blocks so it no longer errors and correctly prints block IDs with their aliases and names, aligning the alias lookup with how get_panel_alias() is structured.
Changes:
- Invert the
get_panel_alias()mapping to usepanel_id -> aliaswhen printing list-blocks output. - Apply the fix in both base CLI and analyze CLI implementations of
list_blocks(). - Add a regression test validating
--list-blocks gfx90aoutput structure and a few known mappings.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| projects/rocprofiler-compute/tests/test_utils.py | Adds regression test coverage for --list-blocks output and a few expected block mappings. |
| projects/rocprofiler-compute/src/rocprof_compute_base.py | Fixes list-blocks alias lookup by using an inverted alias map keyed by block ID. |
| projects/rocprofiler-compute/src/rocprof_compute_analyze/analysis_base.py | Same alias-lookup fix for analyze mode list-blocks output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vedithal-amd
left a comment
vedithal-amd
left a comment
There was a problem hiding this comment.
LGTM with some design comments
| parser.build_dfs(arch_configs=ac, filter_metrics=[], sys_info=sys_info) | ||
|
|
||
| print(f"{'INDEX':<8} {'BLOCK ALIAS':<16} {'BLOCK NAME'}") | ||
| panel_alias_dict = {value: key for key, value in get_panel_alias().items()} |
There was a problem hiding this comment.
We should remove duplicate list_blocks in analyze mode (profile mode is missing this!)
Ideally we should remove all the general options handling out of this if block
, then we can remove duplicate list_blocks handling in analyze mode. General options should not care about the mode IMO.Also I dont know why we mandate --list-metrics with --config-dir, that check should be removed as well.
I will let you decide, if you want to handle it here or in a different PR
|
/AzurePipelines run rocm-ci-caller |
|
No pipelines are associated with this pull request. |
2 participants
Motivation
--list-blocksthrows error and doesn't display blocks.Technical Details
Switch panel alias dict order
JIRA ID
AIPROFCOMP-188
Test Plan
Test Result
Submission Checklist