Skip to content

fix(rocinsight): correct rocprof-sys and rocprof-compute command syntax - #4972

Merged
ammarwa merged 4 commits into
ROCm:rocm21553/pr3-roctx-profiling-scopefrom
ammarwa:rocm21553/pr4-fix-tool-commands
Apr 17, 2026
Merged

ammarwa merged 4 commits into
ROCm:rocm21553/pr3-roctx-profiling-scopefrom
ammarwa:rocm21553/pr4-fix-tool-commands

Conversation

@ammarwa

@ammarwa ammarwa commented Apr 13, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Verified all recommended commands against installed tools and ROCm docs:

  • rocprof-sys (deprecated wrapper) → rocprof-sys-sample (recommended)
  • --trace-gpu-memory (nonexistent) → --trace --device
  • --hip-api-trace stale full_command → --hip-trace
  • Schema tool enum expanded with all valid binaries
  • SCHEMA_CHANGELOG v0.4.0 entry
  • Test files renamed from ticket numbers to descriptive names

Stacking note

Stacks on #4971 → #4970 → #4969 → #4968. GitHub diff includes prior changes. This PR's own changes (10 files, +98/-118):

  • analysis/recommendations.py — rocprof-sys-sample, --hip-trace full_command fix
  • ai_analysis/source_analyzer.py — rocprof-sys-sample commands
  • ai_analysis/docs/analysis-output.schema.json — tool enum, examples
  • ai_analysis/docs/SCHEMA_CHANGELOG.md — v0.4.0 entry
  • Test file renames (3 files)
  • tests/test_analyze.py, tests/test_analyze_schema.py — tool name updates

Merge after #4971 lands.

Test plan

  • 594 tests passing, 0 failures
  • Verified against rocprof-compute --help and rocprof-sys-sample --help

🤖 Generated with Claude Code

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Apr 13, 2026
@ammarwa
ammarwa force-pushed the rocm21553/pr4-fix-tool-commands branch from b85ed30 to 312ba2b Compare April 13, 2026 19:07
@ammarwa
ammarwa marked this pull request as ready for review April 13, 2026 19:33
@ammarwa
ammarwa changed the base branch from develop to rocm21553/pr3-roctx-profiling-scope April 13, 2026 20:41
@ammarwa
ammarwa force-pushed the rocm21553/pr3-roctx-profiling-scope branch from 767146a to cb27d6f Compare April 13, 2026 20:48
@ammarwa
ammarwa force-pushed the rocm21553/pr4-fix-tool-commands branch from 2beb62b to d2cd4dd Compare April 13, 2026 20:48
@ammarwa
ammarwa requested a review from bwelton April 13, 2026 20:53
@ammarwa
ammarwa force-pushed the rocm21553/pr3-roctx-profiling-scope branch from cb27d6f to 76b3325 Compare April 13, 2026 21:00
@ammarwa
ammarwa force-pushed the rocm21553/pr4-fix-tool-commands branch 2 times, most recently from d67b1fb to f9a82b1 Compare April 13, 2026 21:11
@ammarwa
ammarwa force-pushed the rocm21553/pr3-roctx-profiling-scope branch from 5d4f367 to 9d9ef3e Compare April 14, 2026 00:13
ammarwa and others added 4 commits April 13, 2026 19:13
…yntax

Verified all recommended commands against actual tool CLIs and ROCm docs.

rocprof-sys fixes:
- "rocprof-sys" exists as a deprecated wrapper that forwards to
  rocprof-sys-instrument after a 60-second warning/acknowledgement prompt.
  Replaced with "rocprof-sys-sample" which is the recommended non-intrusive
  sampling tool (no binary rewrite needed, no deprecation warning).
- "--trace-gpu-memory" flag does not exist on any rocprof-sys tool — replaced
  with "--trace --device" (-T -D: perfetto trace + device metrics sampling)
- Updated schema tool enum to include all valid binaries:
  rocprof-sys-sample, rocprof-sys-instrument, rocprof-sys-run
- Updated filter logic to match "rocprof-sys-sample" in command deduplication

rocprof-compute fixes:
- Schema example "rocprof-compute -i profile.db --filter kernel_name" had
  invalid flags (-i, --filter not recognized) — replaced with valid syntax
- Verified: --block SQ/TD accepted as raw block names by arg parser
- Verified: --kernel/-k valid for kernel filtering
- Verified: profile subcommand with no flags runs full default collection

594 tests passing, 0 failures.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Documents all schema changes from ROCM-21553:
- New confidence field on recommendations
- New kernel_resources, api_breakdown, roctx_regions top-level fields
- Changed recommendation categories (Compute Bottleneck → context-aware)
- Data-driven estimated_impact values
- Tool command corrections (rocprof-sys-sample, --trace --device)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The structured flags were correctly updated to --hip-trace in PR4,
but the full_command string in the API overhead recommendation was
missed. This caused a copy-paste-ready command to still contain the
invalid flag.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The JSON schema version was not bumped when ROCM-21553 fields
(kernel_resources, api_breakdown, roctx_regions, confidence) were
present. v0.4.0 was already used by ATT Tier 3, so our changes
are v0.5.0. Updated SCHEMA_CHANGELOG to match.

Version progression: 0.1.0 (base) → 0.3.0 (TraceLens) → 0.4.0
(ATT) → 0.5.0 (ROCM-21553). Highest present version wins.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ammarwa
ammarwa force-pushed the rocm21553/pr4-fix-tool-commands branch from f9a82b1 to e0ac9a3 Compare April 14, 2026 00:14
ammarwa added a commit that referenced this pull request Apr 14, 2026
…e output_format param (#4981)

## Summary
- **feat**: Added `private` and `local` to `--llm` CLI choices. These
providers were already supported by the LLM layer (private = enterprise
OpenAI-compatible endpoint, local = Ollama) but not exposed in the
argument parser.
- **deprecate**: `output_format` parameter in `analyze_database()` now
emits `DeprecationWarning` when passed (backwards compatible — not
removed). Callers should use `result.to_json()`, `result.to_webview()`,
or `result.to_text()` instead.

## Stacking note
This PR is based on the ROCM-21553 feature stack (#4968 → #4969 → #4970
→ #4971 → #4972). The GitHub diff against `develop` includes those prior
changes. **This PR's own changes are only 3 files:**
- `rocinsight/analyze.py` — 1 line (--llm choices)
- `rocinsight/ai_analysis/api.py` — output_format deprecation
- `rocinsight/ai_analysis/tests/test_cli_llm_providers.py` — 5 new tests

Merge after #4972 lands, then rebase onto develop for a clean diff.

## Test plan
- [x] 5 new tests, all passing
- [x] Backwards compatible — existing callers passing output_format get
a warning, not an error

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ammarwa added a commit that referenced this pull request Apr 17, 2026
…r 3 serialization (#4978)

## Summary
`to_json()` and `to_webview()` on `AnalysisResult` did not forward
`att_analysis` to `format_analysis_output()`, causing Tier 3 ATT results
to be silently dropped from serialized output despite `analysis_tier=3`.

## Test plan
- [x] 4 new tests verifying att_trace in JSON, schema version 0.4.0, ATT
in webview
- [x] E2E: `analyze_database(att_dir=...)` → `to_json()` → att_trace
present

🤖 Generated with [Claude Code](https://claude.com/claude-code)

## Stacking note
This PR is based on the ROCM-21553 feature stack (#4968-#4972). Its own
change is only: `ai_analysis/api.py` (2 lines adding `att_analysis`
param) + `test_att_serialization.py` (4 tests).

Merge after #4972 lands, then rebase onto develop for a clean diff.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ammarwa
ammarwa merged commit bb99293 into ROCm:rocm21553/pr3-roctx-profiling-scope Apr 17, 2026
20 checks passed
ammarwa added a commit that referenced this pull request Apr 18, 2026
…er AMD docs (#4980)

## Summary
`--kernel-names` is not in current rocprofv3 documentation. AMD docs
specify `--kernel-include-regex` for kernel filtering. Updated all
recommendations, reference guides, interactive session, and tests.
Kernel names with regex special characters are properly escaped with
`re.escape()`.

## Test plan
- [x] 4 new tests: regex flag used, old flag absent, special chars
escaped, filter arg classification
- [x] 0 occurrences of --kernel-names in output

🤖 Generated with [Claude Code](https://claude.com/claude-code)

## Stacking note
This PR is based on the ROCM-21553 feature stack (#4968-#4972). Its own
change is: `recommendations.py`, `interactive.py`, reference guides, and
tests — all replacing `--kernel-names` with `--kernel-include-regex`.

Merge after #4972 lands, then rebase onto develop for a clean diff.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation organization: ROCm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants