From 0ac856237a3af9d5c26da16b3f215b1bbac8f000 Mon Sep 17 00:00:00 2001 From: xuchen-amd Date: Fri, 11 Jul 2025 16:51:55 -0400 Subject: [PATCH] tui user experience improvement (#805) [ROCm/rocprofiler-compute commit: 23664c72f79e97cbf68c6c1b7131b33222409c63] --- .../utils/analyze_config.yaml | 1 + .../rocprof_compute_tui/utils/tui_utils.py | 2 +- .../rocprof_compute_tui/views/main_view.py | 4 +- .../src/rocprof_compute_tui/widgets/charts.py | 2 +- .../widgets/collapsibles.py | 83 ++++++++++++++++--- .../rocprofiler-compute/tests/test_utils.py | 26 ++++-- 6 files changed, 99 insertions(+), 19 deletions(-) diff --git a/projects/rocprofiler-compute/src/rocprof_compute_tui/utils/analyze_config.yaml b/projects/rocprofiler-compute/src/rocprof_compute_tui/utils/analyze_config.yaml index 4e0e348664..daea744094 100644 --- a/projects/rocprofiler-compute/src/rocprof_compute_tui/utils/analyze_config.yaml +++ b/projects/rocprofiler-compute/src/rocprof_compute_tui/utils/analyze_config.yaml @@ -40,6 +40,7 @@ sections: - "1. System Info" - "2. System Speed-of-Light" - "3. Memory Chart" + - "4. Roofline" - title: "🚧 Source Level Analysis" collapsed: true diff --git a/projects/rocprofiler-compute/src/rocprof_compute_tui/utils/tui_utils.py b/projects/rocprofiler-compute/src/rocprof_compute_tui/utils/tui_utils.py index c4179c2c3d..040234a915 100644 --- a/projects/rocprofiler-compute/src/rocprof_compute_tui/utils/tui_utils.py +++ b/projects/rocprofiler-compute/src/rocprof_compute_tui/utils/tui_utils.py @@ -418,7 +418,7 @@ def process_panels_to_dataframes( # Save to CSV if requested if args.df_file_dir: save_dataframe_to_csv(df, table_id_str, table_config, args) - result_structure["roofline"] = roof_plot + result_structure["4. Roofline"] = roof_plot return dict(result_structure) diff --git a/projects/rocprofiler-compute/src/rocprof_compute_tui/views/main_view.py b/projects/rocprofiler-compute/src/rocprof_compute_tui/views/main_view.py index dbd28720de..ba8ce495a7 100644 --- a/projects/rocprofiler-compute/src/rocprof_compute_tui/views/main_view.py +++ b/projects/rocprofiler-compute/src/rocprof_compute_tui/views/main_view.py @@ -122,6 +122,8 @@ class MainView(Horizontal): @work(thread=True) def run_analysis(self) -> None: + self.dfs = {} + if not self.selected_path: error_msg = "No directory selected for analysis" self._update_view(error_msg, LogLevel.ERROR) @@ -237,7 +239,7 @@ class MainView(Horizontal): else: self.app.call_from_thread(self.refresh_results) self.logger.info("Step 8: Analysis completed successfully") - if self.dfs["roofline"]: + if self.dfs.get("4. Roofline"): self.logger.info("Step 8: Roofline data available") else: self.logger.info("Step 8: Roofline data not available") diff --git a/projects/rocprofiler-compute/src/rocprof_compute_tui/widgets/charts.py b/projects/rocprofiler-compute/src/rocprof_compute_tui/widgets/charts.py index c1c791ba12..3cb710f2e2 100644 --- a/projects/rocprofiler-compute/src/rocprof_compute_tui/widgets/charts.py +++ b/projects/rocprofiler-compute/src/rocprof_compute_tui/widgets/charts.py @@ -290,7 +290,7 @@ class RooflinePlot(Static): try: plot_str = "" try: - result = self.df["roofline"] + result = self.df["4. Roofline"] if result: plot_str = str(result) except: diff --git a/projects/rocprofiler-compute/src/rocprof_compute_tui/widgets/collapsibles.py b/projects/rocprofiler-compute/src/rocprof_compute_tui/widgets/collapsibles.py index efe1d97dd3..1970354391 100644 --- a/projects/rocprofiler-compute/src/rocprof_compute_tui/widgets/collapsibles.py +++ b/projects/rocprofiler-compute/src/rocprof_compute_tui/widgets/collapsibles.py @@ -147,7 +147,7 @@ def build_subsection( collapsible = Collapsible(*widgets, title=title, collapsed=collapsed) elif tui_style == "roofline": - if dfs["roofline"]: + if dfs["4. Roofline"]: widget = RooflinePlot(dfs) collapsible = Collapsible(widget, title=title, collapsed=collapsed) else: @@ -172,28 +172,89 @@ def build_dynamic_kernel_sections( ) -> List[Collapsible]: children = [] + def add_warning(message: str): + children.append(Label(message, classes="warning")) + + def validate_data_structure(data, name: str, parent_name: str = None) -> bool: + if data is None: + location = f"'{parent_name}' > '{name}'" if parent_name else f"'{name}'" + add_warning(f"Analysis result for {location} is not available") + return False + + if not isinstance(data, dict): + location = f"'{parent_name}' > '{name}'" if parent_name else f"'{name}'" + add_warning( + f"Analysis result for {location} is not a dictionary type: {type(data)}" + ) + return False + + return True + + def create_safe_widget(subsection_name: str, data: dict, section_name: str): + if not (isinstance(data, dict) and "df" in data): + add_warning( + f"Invalid data structure for '{subsection_name}' in section '{section_name}'" + ) + return None + + try: + df = data["df"] + tui_style = data.get("tui_style") + widget = create_widget_from_data(df, tui_style) + + if widget is None: + add_warning(f"Widget creation returned None for '{subsection_name}'") + return None + + return widget + except Exception as e: + add_warning(f"Failed to create widget for '{subsection_name}': {str(e)}") + return None + + def create_safe_collapsible(widget, title): + try: + return Collapsible(widget, title=title, collapsed=True) + except Exception as e: + add_warning(f"Failed to create collapsible for '{title}': {str(e)}") + return None + try: + if not validate_data_structure(dfs, "analysis result"): + return children + for section_name, subsections in dfs.items(): if section_name in skip_sections: continue + if not validate_data_structure(subsections, section_name): + continue + kernel_children = [] for subsection_name, data in subsections.items(): - if isinstance(data, dict) and "df" in data: - df = data["df"] - tui_style = data.get("tui_style") - widget = create_widget_from_data(df, tui_style) - kernel_children.append( - Collapsible(widget, title=subsection_name, collapsed=True) + try: + widget = create_safe_widget(subsection_name, data, section_name) + if widget: + collapsible = create_safe_collapsible(widget, subsection_name) + if collapsible: + kernel_children.append(collapsible) + except Exception as e: + add_warning( + f"Error processing subsection '{subsection_name}' in section '{section_name}': {str(e)}" ) if kernel_children: - children.append( - Collapsible(*kernel_children, title=section_name, collapsed=True) - ) + try: + section_collapsible = Collapsible( + *kernel_children, title=section_name, collapsed=True + ) + children.append(section_collapsible) + except Exception as e: + add_warning( + f"Failed to create collapsible for section '{section_name}': {str(e)}" + ) except Exception as e: - children.append(Label(f"Error in Kernel Section: {str(e)}", classes="error")) + add_warning(f"Unexpected error in Kernel Section processing: {str(e)}") return children diff --git a/projects/rocprofiler-compute/tests/test_utils.py b/projects/rocprofiler-compute/tests/test_utils.py index 04c209d927..b28e797761 100644 --- a/projects/rocprofiler-compute/tests/test_utils.py +++ b/projects/rocprofiler-compute/tests/test_utils.py @@ -41,8 +41,8 @@ import shutil import subprocess import tempfile from pathlib import Path -from unittest import mock from types import SimpleNamespace +from unittest import mock import pandas as pd import pytest @@ -4997,7 +4997,11 @@ def test_mibench_override_distro_success(tmp_path, monkeypatch): override_binary_path.chmod(0o755) def mock_detect_roofline(mspec): - return {"distro": "override", "path": str(override_binary_path), "rocm_ver": "0.x.x"} + return { + "distro": "override", + "path": str(override_binary_path), + "rocm_ver": "0.x.x", + } subprocess_calls = [] @@ -5451,7 +5455,11 @@ def test_mibench_subprocess_run_failure(tmp_path, monkeypatch): override_binary_path.chmod(0o755) def mock_detect_roofline(mspec): - return {"distro": "override", "path": str(override_binary_path), "rocm_ver": "0.x.x"} + return { + "distro": "override", + "path": str(override_binary_path), + "rocm_ver": "0.x.x", + } def mock_subprocess_run(args, check=True): raise subprocess.CalledProcessError(1, args) @@ -5491,7 +5499,11 @@ def test_mibench_device_string_conversion(tmp_path, monkeypatch): override_binary_path.chmod(0o755) def mock_detect_roofline(mspec): - return {"distro": "override", "path": str(override_binary_path), "rocm_ver": "0.x.x"} + return { + "distro": "override", + "path": str(override_binary_path), + "rocm_ver": "0.x.x", + } subprocess_calls = [] @@ -5600,7 +5612,11 @@ def test_mibench_console_log_called(tmp_path, monkeypatch): override_binary_path.chmod(0o755) def mock_detect_roofline(mspec): - return {"distro": "override", "path": str(override_binary_path), "rocm_ver": "0.x.x"} + return { + "distro": "override", + "path": str(override_binary_path), + "rocm_ver": "0.x.x", + } console_log_calls = []