Repository navigation
feat(rocinsight): add roctx region analysis, profiling scope recommendations - #4971
Merged
ammarwa merged 5 commits intoApr 17, 2026
Conversation
2 tasks done
github-actions
Bot
added
the
documentation
ammarwa
force-pushed
the
rocm21553/pr3-roctx-profiling-scope
branch
from
April 13, 2026 20:24
e1957ea to
c7001c4
Compare
Merged
2 tasks done
ammarwa
changed the base branch from
develop
to
rocm21553/pr2-new-analysis-data
April 13, 2026 20:41
ammarwa
force-pushed
the
rocm21553/pr2-new-analysis-data
branch
from
April 13, 2026 20:48
bb7e625 to
c5bb122
Compare
ammarwa
force-pushed
the
rocm21553/pr3-roctx-profiling-scope
branch
from
April 13, 2026 20:48
767146a to
cb27d6f
Compare
ammarwa
marked this pull request as ready for review
April 13, 2026 20:53
ammarwa
force-pushed
the
rocm21553/pr3-roctx-profiling-scope
branch
from
April 13, 2026 21:00
cb27d6f to
76b3325
Compare
ammarwa
force-pushed
the
rocm21553/pr2-new-analysis-data
branch
from
April 14, 2026 00:13
c5bb122 to
98f2872
Compare
…rof-compute prioritization ROCM-21553 items addressed: - C1: New analyze_roctx_regions() — discovers MARKER_CORE_RANGE_API markers, correlates kernels/memcpy by timestamp overlap, computes per-region and unranged breakdown; returns None gracefully when no markers found - C1: ROCTX RANGE ANALYSIS section in text output (summary table + per-range time breakdown with bar charts); roctx_regions in JSON output - C1: GPU idle warning scoped — when figure-of-merit region shows >90% kernel utilization, suppresses global HIGH idle warning; emits INFO "GPU Utilization (Scoped)" instead - E1: rocprof-compute profile listed first in Kernel Hotspot recommendation commands; rocprofv3 --pmc listed second as alternative - E2: "Profiling Scope" INFO recommendation fires when >50% of traced time is outside annotated roctx regions; includes roctxProfilerPause/Resume code snippet 594 tests passing, 0 failures, 7 new tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…_json() and to_webview() The AnalysisResult.to_json() and to_webview() methods call format_analysis_output() but were not passing the new ROCM-21553 data. This caused the API path (used by interactive mode) to produce JSON without kernel_resources, api_breakdown, or roctx_regions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previous implementation had two issues: 1. Nested ranges (roctxRangePush/Pop): when a kernel sits inside both a parent and child range with equal overlap, the parent was selected first (array order). Now uses range duration as tiebreaker — shorter (more specific/innermost) range wins. 2. Attribution model was undocumented, making it unclear how events are distributed across ranges. New model (innermost-wins, no double-counting): - Each GPU event assigned to exactly ONE range (no double-counting) - Nested ranges: innermost/most-specific range wins ties - Partial overlaps: range with greatest overlap wins - Only overlapping time portion counted for partial overlaps - Parent ranges show work outside child ranges, not inclusive totals Docstring updated with full attribution model description per ROC-TX library spec (roctxRangePush/Pop and roctxRangeStart/Stop semantics). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previous implementation had three accounting errors: 1. Partial overlap lost time: a kernel partially overlapping a marker had only its overlapping portion counted, but the non-overlapping portion vanished (not counted as unranged either). 2. Nested markers double-counted wall time: total_marker_time used sum(durations) which double-counts when ranges nest. A parent [0,100] with child [20,40] summed to 120, not the correct 100. 3. Unranged events missed partially-overlapping ones: _in_marker() returned True for any overlap, so a kernel with 20% overlap and 80% outside was excluded from unranged entirely. Fix uses conservation-correct interval arithmetic: - Unranged time = total_event_time - sum(ranged_portions), ensuring the uncovered portion of partial overlaps is correctly captured - Wall-time coverage uses UNION of marker intervals (merge-sort), not sum of durations, to handle nesting/overlap correctly - _compute_union_coverage() merges overlapping intervals before summing - Guarantees: sum(region_time) + unranged_time == total_time Per ROC-TX library spec: roctxRangePush/Pop (nested) and roctxRangeStart/Stop (cross-thread overlapping) are both supported. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
With innermost-wins ownership, child ranges capture kernel work before parents. But per-region percentages still used the full parent duration as denominator, making parents look artificially idle when children owned most of the work. Example: parent [0,100] with child [20,80]. Parent-exclusive kernel work is 40ns in [0,20]+[80,100]. Old code: kernel_pct = 40/100 = 40%. Fixed: kernel_pct = 40/40 = 100% (exclusive wall = 100 - 60 = 40). Changes: - Pre-compute exclusive_wall_time_ns for each marker by subtracting the union of nested child marker intervals - Use exclusive wall as denominator for kernel_pct/memcpy_pct/overhead_pct - Export exclusive_wall_time_ns in region data for transparency - Added 2 regression tests for nested marker exclusive-vs-inclusive percentages and conservation invariant Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ammarwa
force-pushed
the
rocm21553/pr3-roctx-profiling-scope
branch
from
April 14, 2026 00:13
5d4f367 to
9d9ef3e
Compare
bwelton
approved these changes
Apr 14, 2026
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
…ax (#4972) ## 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 - [x] 594 tests passing, 0 failures - [x] Verified against `rocprof-compute --help` and `rocprof-sys-sample --help` 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ammarwa
merged commit Apr 17, 2026
f98eafe
into
ROCm:rocm21553/pr2-new-analysis-data
20 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ROCM-21553 items C1, E1, E2, 11:
analyze_roctx_regions()— discovers markers, correlates events by overlap (innermost-wins model)rocprof-compute profilelisted first in Kernel Hotspot commandsroctxProfilerPause()/roctxProfilerResume()code snippetStacking note
Stacks on #4970 → #4969 → #4968. GitHub diff includes prior changes. This PR's own changes (8 files, +540/-38):
analysis/core.py—analyze_roctx_regions()with documented attribution modelanalysis/recommendations.py— roctx_regions param, GPU idle scoping, E1 command order, E2 profiler pauseformatters/__init__.py— ROCTX RANGE ANALYSIS sectionformatters/json_fmt.py— roctx_regions in JSONanalyze.py,ai_analysis/api.py— wiringtest_roctx_region_analysis.py— 7 new testsMerge after #4970 lands.
Test plan
🤖 Generated with Claude Code