Skip to content

Add Summary view for compute#670

Open
amokiche-amd wants to merge 9 commits intodevfrom
amokiche/summary-view-from-dev
Open

Add Summary view for compute#670
amokiche-amd wants to merge 9 commits intodevfrom
amokiche/summary-view-from-dev

Conversation

@amokiche-amd
Copy link
Contributor

@amokiche-amd amokiche-amd commented Feb 12, 2026

Motivation

Add Summary view for compute view

image

Technical Details

Added summary view with kernel table and two charts
small refactoring compute tester

Test Plan

Test Result

Submission Checklist

@amokiche-amd amokiche-amd changed the title separate compute views to functions Add Summary view for compute Feb 16, 2026
@amokiche-amd amokiche-amd force-pushed the amokiche/summary-view-from-dev branch 3 times, most recently from 5988826 to 6eafc2b Compare February 20, 2026 15:40
Add pie chart to summary view

add table, piechart and barchart to summary view

Move charts to classes

Move logic to separate classes

Small clean

Fix merge mistakes
Ready for first look

remove compute from cmake

Small refactor

one more refactor
litle improvement

one more

add empty lines in files

delete unnecesary file

delete

Fix cmake
@amokiche-amd amokiche-amd force-pushed the amokiche/summary-view-from-dev branch from e6fc06c to b924c26 Compare February 20, 2026 19:04
@amokiche-amd amokiche-amd marked this pull request as ready for review February 20, 2026 19:10
@drchen-amd drchen-amd marked this pull request as draft February 23, 2026 17:09
@drchen-amd
Copy link
Contributor

Minimum updates for dev compatibility:

  • Removed rocprofvis_compute_summary_view.cpp, integrated contents into rocprofvis_compute_summary.cpp
  • Removed ComputeSummaryView's own workload dropdown, integrated with ComputeSelection.
  • Reverted ComputeTester refactoring. Justification:
    • Splitting it into separate pages makes it less convenient to use. Things like the fetcher are meant to viewed together with the kernel list and available metrics.
    • It will be discarded once view is completed. No point to spend time on refactoring.

@drchen-amd drchen-amd marked this pull request as ready for review February 23, 2026 19:09
Copy link
Contributor

@drchen-amd drchen-amd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something is wrong with the metric dropdown for the charts. The chart looks the same for all the duration, but looking at the table the total duration should be different:

Image Image

Copy link
Contributor

@drchen-amd drchen-amd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chart does not handle long kernel names gracefully:

Image

@drchen-amd
Copy link
Contributor

I think the charts should be styled like system trace summary view charts. It would be nice if all of our charts throughout the app supported a common baseline feature set (hover, tooltip...etc) and style for consistency. You would also get the long name handling for free.

@tomk-amd
Copy link
Collaborator

I think the charts should be styled like system trace summary view charts. It would be nice if all of our charts throughout the app supported a common baseline feature set (hover, tooltip...etc) and style for consistency. You would also get the long name handling for free.

I agree, this was the orginal ask, to make it behave like the chart in systems trace summary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants