From 622592cda438697c4e175557a3133bf2b56df67a Mon Sep 17 00:00:00 2001 From: Hongtao Zhang Date: Fri, 20 Mar 2026 17:47:12 +0000 Subject: [PATCH 1/5] Fix hpblaslt cannot parse the result on V1500. --- .../micro_benchmarks/hipblaslt_function.py | 34 +++++++++++++++---- .../test_hipblaslt_function.py | 31 ++++++++++++++++- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py b/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py index 3feb582d9..62f31d4a1 100644 --- a/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py +++ b/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py @@ -110,7 +110,7 @@ def _process_raw_result(self, cmd_idx, raw_output): lines = raw_output.splitlines() index = None - # Find the line containing 'hipblaslt-Gflops' + # Find the header line containing 'hipblaslt-Gflops' for i, line in enumerate(lines): if 'hipblaslt-Gflops' in line: index = i @@ -119,16 +119,38 @@ def _process_raw_result(self, cmd_idx, raw_output): if index is None: raise ValueError('Line with "hipblaslt-Gflops" not found in the log.') - # Split the line into fields using a comma as the delimiter + # Parse the header to find the column index of 'hipblaslt-Gflops'. + # This is needed because hipBLASLt output format varies across versions: + # - v600 (old): 23 columns, Gflops at index -2 + # - v1500 (new): 34 columns, added a_type/b_type/c_type/scaleA-D/amaxD/ + # bias_type/aux_type/GB_s columns, Gflops at index -3 + # Using header-based lookup ensures compatibility with both formats + # and any future column additions. + header_fields = lines[index].strip().split(',') + # Strip leading markers like '[0]' or '[0]:' from the first header field + header_fields[0] = header_fields[0].split(']')[-1].lstrip(':') + gflops_col = None + for col_idx, col_name in enumerate(header_fields): + if 'hipblaslt-Gflops' in col_name: + gflops_col = col_idx + break + if gflops_col is None: + raise ValueError('Column "hipblaslt-Gflops" not found in header.') + + # Split the data line into fields using a comma as the delimiter fields = lines[index + 1].strip().split(',') - # Check the number of fields and the format of the first two fields - if len(fields) != 23: - raise ValueError('Invalid result') + # Validate that the data line has the same number of columns as the header + if len(fields) != len(header_fields): + raise ValueError( + f'Field count mismatch: header has {len(header_fields)} columns ' + f'but data has {len(fields)} columns' + ) + # batch_count (index 3) and m,n,k (indices 4-6) are stable across all known formats self._result.add_result( f'{self._precision_in_commands[cmd_idx]}_{fields[3]}_{"_".join(fields[4:7])}_flops', - float(fields[-2]) / 1000 + float(fields[gflops_col]) / 1000 ) except BaseException as e: self._result.set_return_code(ReturnCode.MICROBENCHMARK_RESULT_PARSING_FAILURE) diff --git a/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py b/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py index a93d93fbb..2cbc51af5 100644 --- a/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py +++ b/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py @@ -87,6 +87,7 @@ def test_hipblaslt_gemm_result_parsing(self): benchmark._args = SimpleNamespace(shapes=['896,896,896'], in_types=['fp16'], log_raw_data=False) benchmark._result = BenchmarkResult(self.benchmark_name, BenchmarkType.MICRO, ReturnCode.SUCCESS, run_count=1) + # Old format (hipBLASLt v600, 23 columns) example_raw_output = """ hipBLASLt version: 600 hipBLASLt git version: 52776da @@ -101,7 +102,7 @@ def test_hipblaslt_gemm_result_parsing(self): [0]transA,transB,grouped_gemm,batch_count,m,n,k,alpha,lda,stride_a,beta,ldb,stride_b,ldc,stride_c,ldd,stride_d,d_type,compute_type,activation_type,bias_vector,hipblaslt-Gflops,us N,N,0,1,896,896,896,1,896,802816,0,896,802816,896,802816,896,802816,fp16_r,f32_r,none,0, 58624.5, 24.54 """ - # Positive case - valid raw output + # Positive case - valid raw output (old format) self.assertTrue(benchmark._process_raw_result(0, example_raw_output)) self.assertEqual(ReturnCode.SUCCESS, benchmark.return_code) @@ -110,3 +111,31 @@ def test_hipblaslt_gemm_result_parsing(self): # Negative case - invalid raw output self.assertFalse(benchmark._process_raw_result(1, 'HipBLAS API failed')) + + def test_hipblaslt_gemm_result_parsing_new_format(self): + """Test hipblaslt-bench benchmark result parsing with new 34-column format (hipBLASLt v1500+).""" + benchmark = self.get_benchmark() + self.assertTrue(benchmark._preprocess()) + benchmark._args = SimpleNamespace(shapes=['4096,4096,4096'], in_types=['fp16'], log_raw_data=False) + benchmark._result = BenchmarkResult(self.benchmark_name, BenchmarkType.MICRO, ReturnCode.SUCCESS, run_count=1) + + # New format (hipBLASLt v1500, 34 columns) - includes a_type, b_type, c_type, d_type, + # scaleA, scaleB, scaleC, scaleD, amaxD, bias_type, aux_type, and hipblaslt-GB/s columns + example_raw_output_new = """ +hipBLASLt version: 1500 +hipBLASLt git version: 8c69191d +Query device success: there are 1 devices. (Target device ID is 0) +Device ID 0 : gfx942:sramecc+:xnack- +with 205.6 GB memory, max. SCLK 2100 MHz, max. MCLK 1300 MHz, compute capability 9.4 +maxGridDimX 2147483647, sharedMemPerBlock 65.5 KB, maxThreadsPerBlock 1024, warpSize 64 + +Is supported 1 / Total solutions: 1 +[0]:transA,transB,grouped_gemm,batch_count,m,n,k,alpha,lda,stride_a,beta,ldb,stride_b,ldc,stride_c,ldd,stride_d,a_type,b_type,c_type,d_type,compute_type,scaleA,scaleB,scaleC,scaleD,amaxD,activation_type,bias_vector,bias_type,aux_type,hipblaslt-Gflops,hipblaslt-GB/s,us + N,N,0,1,4096,4096,4096,1,4096,16777216,0,4096,16777216,4096,16777216,4096,16777216,f16_r,f16_r,f16_r,f16_r,f32_r,0,0,0,0,0,none,0,f16_r,f16_r,678209,462.62,202.65 +""" + # Positive case - valid raw output (new format) + self.assertTrue(benchmark._process_raw_result(0, example_raw_output_new)) + self.assertEqual(ReturnCode.SUCCESS, benchmark.return_code) + + self.assertEqual(2, len(benchmark.result)) + self.assertEqual(678.209, benchmark.result['fp16_1_4096_4096_4096_flops'][0]) From 7e01df90c715b7e2ff35e63e4598881b96ec40ac Mon Sep 17 00:00:00 2001 From: Hongtao Zhang Date: Sat, 2 May 2026 00:11:56 +0000 Subject: [PATCH 2/5] Address PR review: exact column match, comment cleanup, robust test asserts - Use exact match (col_name.strip() == 'hipblaslt-Gflops') for the FLOPS column lookup, avoiding accidental matches against future names like 'hipblaslt-Gflops-peak'. - Rewrite the inline comment to drop misleading positional index claims and use the correct 'hipblaslt-GB/s' name instead of 'GB_s'. - Replace brittle assertEqual(2, len(...)) and exact float assertEqual with assertIn(...) + assertAlmostEqual(..., places=3) for the new-format test. --- .../micro_benchmarks/hipblaslt_function.py | 14 +++++++------- .../micro_benchmarks/test_hipblaslt_function.py | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py b/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py index 62f31d4a1..563c755db 100644 --- a/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py +++ b/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py @@ -120,18 +120,18 @@ def _process_raw_result(self, cmd_idx, raw_output): raise ValueError('Line with "hipblaslt-Gflops" not found in the log.') # Parse the header to find the column index of 'hipblaslt-Gflops'. - # This is needed because hipBLASLt output format varies across versions: - # - v600 (old): 23 columns, Gflops at index -2 - # - v1500 (new): 34 columns, added a_type/b_type/c_type/scaleA-D/amaxD/ - # bias_type/aux_type/GB_s columns, Gflops at index -3 - # Using header-based lookup ensures compatibility with both formats - # and any future column additions. + # This is needed because the hipBLASLt output format varies across versions: + # - v600 (old): 23 columns. + # - v1500 (new): 34 columns, adding a_type/b_type/c_type/scaleA-D/amaxD/ + # bias_type/aux_type/hipblaslt-GB/s columns. + # Using header-based lookup (plus header/data width validation) ensures + # compatibility across existing formats and resilience to future changes. header_fields = lines[index].strip().split(',') # Strip leading markers like '[0]' or '[0]:' from the first header field header_fields[0] = header_fields[0].split(']')[-1].lstrip(':') gflops_col = None for col_idx, col_name in enumerate(header_fields): - if 'hipblaslt-Gflops' in col_name: + if col_name.strip() == 'hipblaslt-Gflops': gflops_col = col_idx break if gflops_col is None: diff --git a/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py b/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py index 2cbc51af5..a6282ae62 100644 --- a/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py +++ b/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py @@ -137,5 +137,5 @@ def test_hipblaslt_gemm_result_parsing_new_format(self): self.assertTrue(benchmark._process_raw_result(0, example_raw_output_new)) self.assertEqual(ReturnCode.SUCCESS, benchmark.return_code) - self.assertEqual(2, len(benchmark.result)) - self.assertEqual(678.209, benchmark.result['fp16_1_4096_4096_4096_flops'][0]) + self.assertIn('fp16_1_4096_4096_4096_flops', benchmark.result) + self.assertAlmostEqual(678.209, benchmark.result['fp16_1_4096_4096_4096_flops'][0], places=3) From b1323097eed43216b07facce375b5ab757ebcddf Mon Sep 17 00:00:00 2001 From: Hongtao Zhang Date: Sun, 3 May 2026 04:57:45 +0000 Subject: [PATCH 3/5] Address review feedback for hipblaslt parser - Harden first-header-field rank-marker stripping with an anchored regex (re.sub r'^\s*\[\d+\]:?'), so a column whose name happens to contain ']' is not truncated. - Add an explicit IndexError guard before reading lines[index + 1] so hipblaslt-bench output that ends on the header (e.g., crash after printing it) yields a clear ValueError instead of a generic IndexError. --- .../micro_benchmarks/hipblaslt_function.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py b/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py index 563c755db..2c21d01f1 100644 --- a/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py +++ b/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py @@ -4,6 +4,7 @@ """Module of the hipBlasLt GEMM benchmark.""" import os +import re from superbench.common.utils import logger from superbench.benchmarks import BenchmarkRegistry, Platform, ReturnCode @@ -127,8 +128,10 @@ def _process_raw_result(self, cmd_idx, raw_output): # Using header-based lookup (plus header/data width validation) ensures # compatibility across existing formats and resilience to future changes. header_fields = lines[index].strip().split(',') - # Strip leading markers like '[0]' or '[0]:' from the first header field - header_fields[0] = header_fields[0].split(']')[-1].lstrip(':') + # Strip leading rank markers like '[0]' or '[0]:' from the first header field. + # Use a regex anchored at the start so a column name that legitimately contains + # ']' (unlikely, but defensive) is not truncated. + header_fields[0] = re.sub(r'^\s*\[\d+\]:?', '', header_fields[0]) gflops_col = None for col_idx, col_name in enumerate(header_fields): if col_name.strip() == 'hipblaslt-Gflops': @@ -137,6 +140,11 @@ def _process_raw_result(self, cmd_idx, raw_output): if gflops_col is None: raise ValueError('Column "hipblaslt-Gflops" not found in header.') + # Ensure a data line follows the header (e.g., hipblaslt-bench may have + # crashed after printing the header). + if index + 1 >= len(lines): + raise ValueError('Data line missing after "hipblaslt-Gflops" header.') + # Split the data line into fields using a comma as the delimiter fields = lines[index + 1].strip().split(',') From b9010745146a38f085e7b280dc83289f60502d91 Mon Sep 17 00:00:00 2001 From: Hongtao Zhang Date: Wed, 20 May 2026 20:38:26 +0000 Subject: [PATCH 4/5] Address Copilot review: header-driven lookup for batch_count/m/n/k Resolves the remaining unresolved Copilot review comment on PR #791 (comment id 3177652631): the previous fix only resolved the hipblaslt-Gflops column by header name; the metric key still used fixed positions 3-6 for batch_count/m/n/k, defeating the forward-compatibility claim. Changes: - Build a column-name -> index map from the parsed header (first-occurrence-wins via dict.setdefault) and resolve every key column (batch_count, m, n, k, hipblaslt-Gflops) by name. - Raise ValueError listing all missing required columns so unknown output formats fail loudly instead of silently producing a wrong metric key. - Strip whitespace from each selected data value before building the metric key, so any CSV padding does not bleed into the key. - Replace the misleading 'indices 3/4-6 are stable' comment with a note explaining that all key fields are header-driven. Tests: - Add forward-compat positive test that inserts an extra column before batch_count and pads data values with whitespace, asserting the correct key is produced and that no key derived from the wrong positional field leaks through. - Add negative test asserting MICROBENCHMARK_RESULT_PARSING_FAILURE when a required column (batch_count) is missing from the header. --- .../micro_benchmarks/hipblaslt_function.py | 38 ++++++++----- .../test_hipblaslt_function.py | 55 +++++++++++++++++++ 2 files changed, 78 insertions(+), 15 deletions(-) diff --git a/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py b/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py index 2c21d01f1..6c708b3c2 100644 --- a/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py +++ b/superbench/benchmarks/micro_benchmarks/hipblaslt_function.py @@ -120,25 +120,26 @@ def _process_raw_result(self, cmd_idx, raw_output): if index is None: raise ValueError('Line with "hipblaslt-Gflops" not found in the log.') - # Parse the header to find the column index of 'hipblaslt-Gflops'. - # This is needed because the hipBLASLt output format varies across versions: - # - v600 (old): 23 columns. - # - v1500 (new): 34 columns, adding a_type/b_type/c_type/scaleA-D/amaxD/ - # bias_type/aux_type/hipblaslt-GB/s columns. - # Using header-based lookup (plus header/data width validation) ensures - # compatibility across existing formats and resilience to future changes. + # Parse the header and resolve every key column (batch_count/m/n/k/hipblaslt-Gflops) + # by name. This keeps the parser forward-compatible across known and future + # hipBLASLt output formats (v600: 23 columns; v1500: 34 columns with extra + # a_type/b_type/c_type/scaleA-D/amaxD/bias_type/aux_type/hipblaslt-GB/s), + # without relying on any fixed column position. header_fields = lines[index].strip().split(',') # Strip leading rank markers like '[0]' or '[0]:' from the first header field. # Use a regex anchored at the start so a column name that legitimately contains # ']' (unlikely, but defensive) is not truncated. header_fields[0] = re.sub(r'^\s*\[\d+\]:?', '', header_fields[0]) - gflops_col = None + + # Build a name -> column-index map (first occurrence wins for any duplicates). + col_idx_by_name = {} for col_idx, col_name in enumerate(header_fields): - if col_name.strip() == 'hipblaslt-Gflops': - gflops_col = col_idx - break - if gflops_col is None: - raise ValueError('Column "hipblaslt-Gflops" not found in header.') + col_idx_by_name.setdefault(col_name.strip(), col_idx) + + required_columns = ['batch_count', 'm', 'n', 'k', 'hipblaslt-Gflops'] + missing_columns = [c for c in required_columns if c not in col_idx_by_name] + if missing_columns: + raise ValueError(f'Required column(s) not found in header: {missing_columns}.') # Ensure a data line follows the header (e.g., hipblaslt-bench may have # crashed after printing the header). @@ -155,9 +156,16 @@ def _process_raw_result(self, cmd_idx, raw_output): f'but data has {len(fields)} columns' ) - # batch_count (index 3) and m,n,k (indices 4-6) are stable across all known formats + # Resolve every key value by header name and strip whitespace from each, so + # any padding around CSV values does not bleed into the metric key. + batch_count = fields[col_idx_by_name['batch_count']].strip() + m_val = fields[col_idx_by_name['m']].strip() + n_val = fields[col_idx_by_name['n']].strip() + k_val = fields[col_idx_by_name['k']].strip() + gflops_col = col_idx_by_name['hipblaslt-Gflops'] + self._result.add_result( - f'{self._precision_in_commands[cmd_idx]}_{fields[3]}_{"_".join(fields[4:7])}_flops', + f'{self._precision_in_commands[cmd_idx]}_{batch_count}_{m_val}_{n_val}_{k_val}_flops', float(fields[gflops_col]) / 1000 ) except BaseException as e: diff --git a/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py b/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py index a6282ae62..7f9d1e573 100644 --- a/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py +++ b/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py @@ -139,3 +139,58 @@ def test_hipblaslt_gemm_result_parsing_new_format(self): self.assertIn('fp16_1_4096_4096_4096_flops', benchmark.result) self.assertAlmostEqual(678.209, benchmark.result['fp16_1_4096_4096_4096_flops'][0], places=3) + + def test_hipblaslt_gemm_result_parsing_future_format_with_inserted_column(self): + """Test that the parser is forward-compatible when a new column is inserted before batch_count. + + This proves the metric key is built purely from header-named columns, not fixed + positions, so reordering or inserting columns in a future hipBLASLt release does + not silently produce a wrong metric key. + """ + benchmark = self.get_benchmark() + self.assertTrue(benchmark._preprocess()) + benchmark._args = SimpleNamespace(shapes=['4096,4096,4096'], in_types=['fp16'], log_raw_data=False) + benchmark._result = BenchmarkResult(self.benchmark_name, BenchmarkType.MICRO, ReturnCode.SUCCESS, run_count=1) + + # Synthetic future format: a new column 'fake_new_col' is inserted before batch_count, + # and the data values for the key fields are padded with whitespace to confirm that + # individual field values are stripped before being used to build the metric key. + # A minimal header is used so the padded data line stays within the 120-column limit. + example_raw_output_future = """ +hipBLASLt version: 9999 +Is supported 1 / Total solutions: 1 +[0]:transA,transB,fake_new_col,batch_count,m,n,k,hipblaslt-Gflops,us +N,N,FAKE, 1 , 4096 , 4096 , 4096 ,678209,202.65 +""" + self.assertTrue(benchmark._process_raw_result(0, example_raw_output_future)) + self.assertEqual(ReturnCode.SUCCESS, benchmark.return_code) + + # The correct, header-driven key must be present with the correct value. + self.assertIn('fp16_1_4096_4096_4096_flops', benchmark.result) + self.assertAlmostEqual(678.209, benchmark.result['fp16_1_4096_4096_4096_flops'][0], places=3) + + # No key derived from the wrong (positional) field should leak through. + for key in benchmark.result: + self.assertNotIn('FAKE', key) + self.assertNotIn('fake_new_col', key) + + def test_hipblaslt_gemm_result_parsing_missing_required_column(self): + """Test that the parser fails loudly when a required key column (e.g. batch_count) is missing. + + Failing surfaces unknown output formats explicitly instead of silently producing + a wrong metric key. + """ + benchmark = self.get_benchmark() + self.assertTrue(benchmark._preprocess()) + benchmark._args = SimpleNamespace(shapes=['896,896,896'], in_types=['fp16'], log_raw_data=False) + benchmark._result = BenchmarkResult(self.benchmark_name, BenchmarkType.MICRO, ReturnCode.SUCCESS, run_count=1) + + # batch_count is removed from both the header and the data line. + example_raw_output_missing_col = """ +hipBLASLt version: 600 +Is supported 1 / Total solutions: 1 +[0]transA,transB,grouped_gemm,m,n,k,alpha,lda,stride_a,beta,ldb,stride_b,ldc,stride_c,ldd,stride_d,d_type,compute_type,activation_type,bias_vector,hipblaslt-Gflops,us +N,N,0,896,896,896,1,896,802816,0,896,802816,896,802816,896,802816,fp16_r,f32_r,none,0, 58624.5, 24.54 +""" + self.assertFalse(benchmark._process_raw_result(0, example_raw_output_missing_col)) + self.assertEqual(ReturnCode.MICROBENCHMARK_RESULT_PARSING_FAILURE, benchmark.return_code) From cff175ab71733296930796bdc0cfbe81b84f3e28 Mon Sep 17 00:00:00 2001 From: Hongtao Zhang Date: Wed, 20 May 2026 21:44:57 +0000 Subject: [PATCH 5/5] Address Copilot review: harden v600 result-parsing test assertions Resolves Copilot inline review comment id 3277063504 on PR #791. The v1500 and future-format tests already followed the assertIn + assertAlmostEqual pattern after the previous review round; this commit applies the same hardening to the v600 positive test that was missed. - Drop self.assertEqual(2, len(benchmark.result)) which coupled the test to the always-present return_code metric instead of the behavior under test. - Replace self.assertEqual(58.6245, ...) (exact float equality) with self.assertIn(...) + self.assertAlmostEqual(..., places=4) so the numeric check is not flaky on floating-point representation. places=4 matches the four decimal places in the expected fixture value. --- tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py b/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py index 7f9d1e573..484790755 100644 --- a/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py +++ b/tests/benchmarks/micro_benchmarks/test_hipblaslt_function.py @@ -106,8 +106,8 @@ def test_hipblaslt_gemm_result_parsing(self): self.assertTrue(benchmark._process_raw_result(0, example_raw_output)) self.assertEqual(ReturnCode.SUCCESS, benchmark.return_code) - self.assertEqual(2, len(benchmark.result)) - self.assertEqual(58.6245, benchmark.result['fp16_1_896_896_896_flops'][0]) + self.assertIn('fp16_1_896_896_896_flops', benchmark.result) + self.assertAlmostEqual(58.6245, benchmark.result['fp16_1_896_896_896_flops'][0], places=4) # Negative case - invalid raw output self.assertFalse(benchmark._process_raw_result(1, 'HipBLAS API failed'))