From 706a8382a50f9e37357a05b0aca57b15bd0adba6 Mon Sep 17 00:00:00 2001 From: jamessiddeley-amd Date: Fri, 12 Dec 2025 17:06:37 -0500 Subject: [PATCH] [rocprof-compute] added graceful exit with corrupt roofline.csv in profile and analyze mode (#1811) * added graceful errors/exit in profile/analyze roofline.csv * edit if statement truth * restore if statement truth (roofline_csv needs at least 2 rows) * addressed comments and skipped showing roof metrics when data invalid * fix workload merge * changed warning to error * removed redundant variable definition * added roofline csv validate check in TUI * add test cases to test validation function * ruff format * simplified TUI roofline handling --- .../rocprof_compute_analyze/analysis_base.py | 27 ++- .../src/rocprof_compute_soc/soc_base.py | 29 ++- .../src/rocprof_compute_tui/analysis_tui.py | 7 +- projects/rocprofiler-compute/src/roofline.py | 21 +- .../src/utils/roofline_calc.py | 185 ++++++++++++++---- projects/rocprofiler-compute/src/utils/tty.py | 37 +++- .../rocprofiler-compute/tests/test_utils.py | 45 +++++ 7 files changed, 288 insertions(+), 63 deletions(-) diff --git a/projects/rocprofiler-compute/src/rocprof_compute_analyze/analysis_base.py b/projects/rocprofiler-compute/src/rocprof_compute_analyze/analysis_base.py index abb8021eba..34fde73ce2 100644 --- a/projects/rocprofiler-compute/src/rocprof_compute_analyze/analysis_base.py +++ b/projects/rocprofiler-compute/src/rocprof_compute_analyze/analysis_base.py @@ -45,6 +45,7 @@ from utils.logger import ( console_warning, demarcate, ) +from utils.roofline_calc import validate_roofline_csv from utils.utils import ( get_panel_alias, get_uuid, @@ -276,11 +277,27 @@ class OmniAnalyze_Base: if sysinfo_path: w.sys_info = file_io.load_sys_info(f"{sysinfo_path}/sysinfo.csv") if not getattr(args, "no_roof", False): - try: - roofline_df = pd.read_csv(f"{sysinfo_path}/roofline.csv") - w.roofline_peaks = roofline_df - except FileNotFoundError: - console_warning("roofline.csv not found.") + # Validate roofline CSV before loading + + is_valid, error_msg = validate_roofline_csv(sysinfo_path) + + if is_valid: + try: + roofline_df = pd.read_csv(f"{sysinfo_path}/roofline.csv") + w.roofline_peaks = roofline_df + except Exception as e: + console_error( + "roofline", + f"Failed to load roofline.csv: {e}", + exit=False, + ) + w.roofline_peaks = pd.DataFrame() + else: + console_error( + "roofline", + f"Roofline analysis skipped: {error_msg}", + exit=False, + ) w.roofline_peaks = pd.DataFrame() else: w.roofline_peaks = pd.DataFrame() diff --git a/projects/rocprofiler-compute/src/rocprof_compute_soc/soc_base.py b/projects/rocprofiler-compute/src/rocprof_compute_soc/soc_base.py index 40bc5c9be7..0735bb618f 100644 --- a/projects/rocprofiler-compute/src/rocprof_compute_soc/soc_base.py +++ b/projects/rocprofiler-compute/src/rocprof_compute_soc/soc_base.py @@ -46,6 +46,7 @@ from utils.logger import ( ) from utils.mi_gpu_spec import mi_gpu_specs from utils.parser import BUILD_IN_VARS, SUPPORTED_DENOM +from utils.roofline_calc import validate_roofline_csv from utils.specs import MachineSpecs from utils.utils import ( METRIC_ID_RE, @@ -669,16 +670,36 @@ class OmniSoC_Base: pmc_path = Path(self.get_args().path) / "pmc_perf.csv" if not pmc_path.is_file(): - console_warning( - "Incomplete or missing profiling data. Skipping roofline." + console_error( + "roofline", + "Incomplete or missing profiling data. Skipping roofline.", + exit=False, ) return console_log( "roofline", f"Checking for roofline.csv in {self.get_args().path}" ) if not (Path(self.get_args().path) / "roofline.csv").is_file(): - result = benchmark.run_on_devices([self.get_args().device]) - benchmark.dump_csv(result, f"{self.get_args().path}/roofline.csv") + try: + result = benchmark.run_on_devices([self.get_args().device]) + benchmark.dump_csv(result, f"{self.get_args().path}/roofline.csv") + except Exception as e: + console_error( + "roofline", + f"Benchmark execution failed: {e}. Skipping roofline.", + exit=False, + ) + return + + # Validate roofline.csv before post-processing + is_valid, error_msg = validate_roofline_csv(self.get_args().path) + if not is_valid: + console_error( + "roofline", + f"Roofline post-processing skipped: {error_msg}", + exit=False, + ) + return self.roofline_obj.post_processing() diff --git a/projects/rocprofiler-compute/src/rocprof_compute_tui/analysis_tui.py b/projects/rocprofiler-compute/src/rocprof_compute_tui/analysis_tui.py index 62668e7361..b9b1b89932 100644 --- a/projects/rocprofiler-compute/src/rocprof_compute_tui/analysis_tui.py +++ b/projects/rocprofiler-compute/src/rocprof_compute_tui/analysis_tui.py @@ -138,9 +138,10 @@ class tui_analysis(OmniAnalyze_Base): else sys_info ) - roofline_path = Path(self.path) / "roofline.csv" - if roofline_path.is_file() and not getattr(self.args, "no_roof", False): - w.roofline_peaks = pd.read_csv(roofline_path) + # NOTE: Roofline is not yet supported in TUI. Keep roofline_peaks empty. + # When roofline support is added to TUI, this should use validate_roofline_csv() + # similar to analysis_base.py. + w.roofline_peaks = pd.DataFrame() w.avail_ips = w.sys_info["ip_blocks"].item().split("|") w.dfs = copy.deepcopy(self._arch_configs[arch].dfs) diff --git a/projects/rocprofiler-compute/src/roofline.py b/projects/rocprofiler-compute/src/roofline.py index 57f47bc67d..fa2fa6f8f3 100644 --- a/projects/rocprofiler-compute/src/roofline.py +++ b/projects/rocprofiler-compute/src/roofline.py @@ -578,6 +578,15 @@ class Roofline: ) console_debug("roofline", f"Ceiling data:\n{self.__ceiling_data}") + if all( + v is None or all(x is None for x in v) for v in self.__ceiling_data.values() + ): + console_warning( + "Unable to generate roofline plot due to missing or corrupted " + "benchmark data. Returning empty figure." + ) + return fig if fig is not None else go.Figure() + ops_flops = "OP" if dtype.startswith("I") else "FLOP" subplot_kwargs = {"row": subplot_row, "col": 1} if subplot_row else {} @@ -1193,6 +1202,15 @@ class Roofline: console_log("roofline", f"{roofline_csv} does not exist") return + if ( + workload + and hasattr(workload, "roofline_peaks") + and workload.roofline_peaks.empty + ): + # CSV validation failed earlier, skip plot generation + console_warning("roofline", "Skipping plot generation") + return None + # if workload is detected, utilize Roofline yamls. # If not, fallback to legacy calc_ai if workload and config and arch_config: @@ -1225,9 +1243,6 @@ class Roofline: roofline_parameters=self.__run_parameters, dtype=dtype ) - console_debug(f"AI data: {self.__ai_data}") - console_debug(f"Kernel names: {self.__ai_data.get('kernelNames', [])}") - self.roof_setup() # Check proper datatype input - takes single str diff --git a/projects/rocprofiler-compute/src/utils/roofline_calc.py b/projects/rocprofiler-compute/src/utils/roofline_calc.py index 8825ebbe82..851d62a4d0 100644 --- a/projects/rocprofiler-compute/src/utils/roofline_calc.py +++ b/projects/rocprofiler-compute/src/utils/roofline_calc.py @@ -31,7 +31,7 @@ from typing import Any, Optional, Union import pandas as pd from utils import schema -from utils.logger import console_debug, console_warning +from utils.logger import console_debug, console_error, console_warning from utils.parser import apply_filters, eval_metric from utils.specs import MachineSpecs @@ -240,15 +240,30 @@ def calc_ceilings( peak_ops = 0.0 if dtype in PEAK_OPS_DATATYPES: - peak_ops = float( - benchmark_data[f"{dtype}{ops_flops}"][roofline_parameters["device_id"]] - ) + try: + peak_ops = float( + benchmark_data[f"{dtype}{ops_flops}"][roofline_parameters["device_id"]] + ) + except KeyError: + console_warning( + f"Missing benchmark data for {dtype}{ops_flops} in benchmark_results. " + "Skipping peak operations calculation. This may indicate incomplete or " + "corrupted benchmark data." + ) + return GraphPoints.empty().__dict__ for cache_level in cache_hierarchy: # Plot BW line - console_debug("roofline", f"Current cache level is {cache_level}") curr_bw = f"{cache_level}Bw" - peak_bw = float(benchmark_data[curr_bw][roofline_parameters["device_id"]]) + try: + peak_bw = float(benchmark_data[curr_bw][roofline_parameters["device_id"]]) + except KeyError: + console_warning( + f"Missing benchmark data for {curr_bw} in benchmark_results. " + f"Skipping {cache_level} cache level. This may indicate incomplete or " + "corrupted benchmark data." + ) + continue x1 = float(XMIN) y1 = float(XMIN) * peak_bw @@ -265,13 +280,21 @@ def calc_ceilings( if dtype in MFMA_DATATYPES: target_precision = dtype if dtype.startswith("I") else f"F{dtype[2:]}" - peak_mfma = float( - benchmark_data[f"MFMA{target_precision}{ops_flops}"][ - roofline_parameters["device_id"] - ] - ) - x2_mfma = peak_mfma / peak_bw - y2_mfma = peak_mfma + try: + peak_mfma = float( + benchmark_data[f"MFMA{target_precision}{ops_flops}"][ + roofline_parameters["device_id"] + ] + ) + x2_mfma = peak_mfma / peak_bw + y2_mfma = peak_mfma + except KeyError: + console_warning( + f"Missing benchmark data for " + f"MFMA{target_precision}{ops_flops} in benchmark_results. " + f"Skipping MFMA calculations for {cache_level} cache level. " + "This may indicate incomplete or corrupted benchmark data." + ) # Check which peak is higher for formatting bandwidth lines if y2_mfma > y1_mfma: # peak_mfma @@ -281,11 +304,6 @@ def calc_ceilings( peak_x = x1_mfma peak_y = y1_mfma - # These are the points to use: - console_debug("roofline", "coordinate points:") - console_debug(f"x = [{x1}, {peak_x}]") - console_debug(f"y = [{y1}, {peak_y}]") - cache_key = cache_level.lower() graph_points[cache_key].extend([[x1, peak_x], [y1, peak_y], peak_bw]) @@ -296,7 +314,6 @@ def calc_ceilings( # Plot FMA roof x0 = min(x2, dynamic_xmax) if x2 < dynamic_xmax else dynamic_xmax - console_debug(f"FMA ROOF [{x0}, {dynamic_xmax}], [{peak_ops},{peak_ops}]") graph_points["valu"].extend([ [x0, dynamic_xmax], [peak_ops, peak_ops], @@ -307,9 +324,6 @@ def calc_ceilings( if dtype in MFMA_DATATYPES: # assert that mfma has been assigned x0_mfma = min(x2_mfma, dynamic_xmax) if x2_mfma < dynamic_xmax else dynamic_xmax - console_debug( - f"MFMA ROOF [{x0_mfma}, {dynamic_xmax}], [{peak_mfma},{peak_mfma}]" - ) graph_points["mfma"].extend([ [x0_mfma, dynamic_xmax], [peak_mfma, peak_mfma], @@ -397,14 +411,6 @@ def calc_ai_analyze( config=config, ) - # DEBUG - if 402 in kernel_dfs: - console_debug("roofline", f"Table 402 for kernel {kernel_id}:") - for idx, row in kernel_dfs[402].iterrows(): - console_debug( - "roofline", f" {row.get('Metric', '')}: {row.get('Value', '')}" - ) - ai_hbm = ai_l2 = ai_l1 = performance = 0 if 402 in kernel_dfs: @@ -456,7 +462,6 @@ def calc_ai_analyze( } console_debug("roofline", f"Generated {len(plot_points.kernelNames)} plot points") - console_debug("roofline", f"Plot points: {plot_points}") return plot_points.__dict__ @@ -792,10 +797,15 @@ def calc_ai_profile( return intensity_points -def construct_roof( - roofline_parameters: dict[str, Any], dtype: str, ai_data: Optional[dict] = None -) -> dict[str, list[Union[list[float], float, None]]]: - workload_dir = roofline_parameters.get("workload_dir") +def validate_roofline_csv(workload_dir: Union[str, Path, list]) -> tuple[bool, str]: + """ + Validate roofline.csv exists and has consistent structure. + + Returns: + tuple: (is_valid, error_message) + is_valid=True if CSV is valid, False otherwise + error_message contains description if invalid + """ if isinstance(workload_dir, list): base_dir = ( workload_dir[0][0] @@ -807,10 +817,68 @@ def construct_roof( benchmark_results = Path(base_dir) / "roofline.csv" - # ----------------------------------------------------- - # Initialize roofline data dictionary from roofline.csv - # ----------------------------------------------------- - # TODO: consider changing this to an ordered dict for consistency over py versions + # Check if file exists + if not benchmark_results.exists(): + return False, f"Benchmark results file not found: {benchmark_results}" + + # Validate CSV structure + try: + with open(benchmark_results) as csvfile: + csv_reader = csv.reader(csvfile, delimiter=",") + row_count = 0 + num_headers = 0 + + for row in csv_reader: + if row_count == 0: + num_headers = len(row) - 1 + if num_headers <= 0: + return ( + False, + "Empty or invalid header row in benchmark_results", + ) + else: + if len(row) - 1 != num_headers: + return ( + False, + f"Inconsistent row length in benchmark_results at " + f"row {row_count + 1}. " + f"Expected {num_headers + 1} columns, " + f"found {len(row)}. " + "Roofline data appears corrupted or incomplete.", + ) + row_count += 1 + + if row_count < 2: + return ( + False, + f"Insufficient data in benchmark_results. " + f"Found {row_count} rows (need at least 2)." + f" Roofline data appears corrupted or incomplete.", + ) + except Exception as e: + return False, f"Failed to read benchmark_results: {e}" + + return True, "" + + +def construct_roof( + roofline_parameters: dict[str, Any], dtype: str, ai_data: Optional[dict] = None +) -> dict[str, list[Union[list[float], float, None]]]: + workload_dir = roofline_parameters.get("workload_dir") + + # Normalize workload_dir to extract base directory + if isinstance(workload_dir, list): + base_dir = ( + workload_dir[0][0] + if isinstance(workload_dir[0], (list, tuple)) + else workload_dir[0] + ) + else: + base_dir = workload_dir + + benchmark_results = Path(base_dir) / "roofline.csv" + + # Initialize benchmark data dictionary from roofline.csv benchmark_data: dict[str, list[str]] = {} headers: list[str] = [] @@ -820,7 +888,7 @@ def construct_roof( row_count = 0 for row in csv_reader: - row.pop(0) # remove devID + row.pop(0) # Remove first column (Device ID) if row_count == 0: headers = row for header in headers: @@ -830,9 +898,44 @@ def construct_roof( benchmark_data[key].append(row[i]) row_count += 1 except Exception as e: - console_debug("roofline", f"Failed to read benchmark results: {e}") + console_error( + "roofline", + f"Failed to read benchmark results from {base_dir}: {e}", + exit=False, + ) return GraphPoints.empty().__dict__ + # ------------------ + # Validate benchmark data completeness + # ------------------ + ops_flops = "Ops" if dtype.startswith("I") else "Flops" + expected_columns = [] + + if dtype in PEAK_OPS_DATATYPES: + expected_columns.append(f"{dtype}{ops_flops}") + + cache_hierarchy = ( + CACHE_HIERARCHY + if roofline_parameters["mem_level"] == "ALL" + else roofline_parameters["mem_level"] + ) + for cache_level in cache_hierarchy: + expected_columns.append(f"{cache_level}Bw") + + if dtype in MFMA_DATATYPES: + target_precision = dtype if dtype.startswith("I") else f"F{dtype[2:]}" + expected_columns.append(f"MFMA{target_precision}{ops_flops}") + + # Check for missing expected columns + missing_columns = [col for col in expected_columns if col not in benchmark_data] + if missing_columns: + console_warning( + f"Missing expected columns in roofline.csv for datatype {dtype}: " + f"{', '.join(missing_columns)}. " + "The roofline plot may be incomplete. Consider regenerating " + "benchmark data or cleaning the directory and re-running the analysis." + ) + # ------------------ # Generate Roofline # ------------------ diff --git a/projects/rocprofiler-compute/src/utils/tty.py b/projects/rocprofiler-compute/src/utils/tty.py index 396abc13b7..3a467ef707 100644 --- a/projects/rocprofiler-compute/src/utils/tty.py +++ b/projects/rocprofiler-compute/src/utils/tty.py @@ -155,9 +155,18 @@ def is_roofline_shown( ): return False - print(f"\n{'=' * 80}", file=output) + # Check if any run has valid roofline data (already validated in analysis_base.py) + # This check determines whether to display roofline section in the output report + if not any( + hasattr(workload, "roofline_peaks") and not workload.roofline_peaks.empty + for workload in runs.values() + ): + # roofline_peaks is empty, meaning CSV validation failed earlier. + # Skip displaying this section entirely (error already logged). + return False + + print(f"\n{'-' * 80}", file=output) print("4. Roofline", file=output) - print("=" * 80, file=output) # Display roofline metrics for each run for run_path, workload in runs.items(): @@ -166,7 +175,6 @@ def is_roofline_shown( "\n(4.1) Per-Kernel Roofline Metrics and (4.2) AI Plot Points", file=output, ) - print("-" * 80, file=output) kernel_top_df = workload.dfs.get(1, pd.DataFrame()) if not kernel_top_df.empty: @@ -490,15 +498,30 @@ def show_all( if len(args.path) > 1 and panel_id in config.HIDDEN_SECTIONS: continue - if panel_id == 400 and not is_roofline_shown( - args, runs, output, panel, roof_plot, hidden_cols - ): - continue + # Handle roofline panel (400) with custom display logic, then skip normal + # table processing to prevent duplicate printing. + if panel_id == 400: + if is_roofline_shown(args, runs, output, panel, roof_plot, hidden_cols): + continue panel_content = "" # store content of all data_source from one panel for data_source in panel["data source"]: for table_type, table_config in data_source.items(): + # Skip roofline tables (401, 402) if roofline data is invalid + if table_config["id"] in [401, 402]: + has_valid_roofline = any( + hasattr(workload, "roofline_peaks") + and not workload.roofline_peaks.empty + for workload in runs.values() + ) + if not has_valid_roofline: + console_warning( + f"Not showing Roofline table {table_config['id']} " + "due to invalid roofline data." + ) + continue + # Block-filter logic: # - If analysis used --filter-metrics, ignore profiling block filters # - If profiling had block filters, only show selected tables/panels diff --git a/projects/rocprofiler-compute/tests/test_utils.py b/projects/rocprofiler-compute/tests/test_utils.py index 89b027cc5f..3dafe53621 100644 --- a/projects/rocprofiler-compute/tests/test_utils.py +++ b/projects/rocprofiler-compute/tests/test_utils.py @@ -7967,3 +7967,48 @@ def test_merge_counters_iteration_multiplex(): assert "Mean_Time" in column_headers assert "Median_Time" in column_headers assert len(result) == 3 + + +# ============================================================================= +# validate_roofline_csv TESTS +# ============================================================================= + + +def test_validate_roofline_csv_valid(): + """ + Test validate_roofline_csv returns True for a valid roofline.csv file. + Creates a temporary directory with a properly formatted CSV. + """ + from utils.roofline_calc import validate_roofline_csv + + with tempfile.TemporaryDirectory() as tmpdir: + csv_path = Path(tmpdir) / "roofline.csv" + csv_path.write_text( + "device,HBMBw,L2Bw,L1Bw,FP32Flops,FP64Flops\n" + "0,1000.0,2000.0,3000.0,4000.0,5000.0\n" + ) + + is_valid, error_msg = validate_roofline_csv(tmpdir) + + assert is_valid is True + assert error_msg == "" + + +def test_validate_roofline_csv_invalid_inconsistent_columns(): + """ + Test validate_roofline_csv returns False for a CSV with inconsistent row lengths. + This simulates corrupted or incomplete benchmark data. + """ + from utils.roofline_calc import validate_roofline_csv + + with tempfile.TemporaryDirectory() as tmpdir: + csv_path = Path(tmpdir) / "roofline.csv" + csv_path.write_text( + "device,HBMBw,L2Bw,L1Bw,FP32Flops,FP64Flops\n0,1000.0,2000.0,3000.0\n" + ) + + is_valid, error_msg = validate_roofline_csv(tmpdir) + + assert is_valid is False + assert "Inconsistent row length" in error_msg + assert "row 2" in error_msg