Use Exceptions in onnx importer to avoid importing broken test cases. (…

…#9)

A few of these cases were allowing errors to slip through as broken test
cases. Rather than rely on checking for `None` or watching the logs,
this uses Exceptions as error handling.

As a result of fixing these oversights, some broken test cases are
deleted. I'm not sure how `update_config_xfails.py` will handle the list
of tests changing.

Logs from running the import script:
https://gist.github.com/ScottTodd/bd327bc7e95485b12a9f9ebdcddf0bc2

.github/workflows/test_onnx_ops.yml

@@ -58,15 +58,13 @@ jobs:
             --report-log=/tmp/onnx_ops_cpu_logs.json \
             --config-files=${CONFIG_FILE_PATH}
       - name: "Updating config file with latest XFAIL lists"
-        if: failure()
         run: |
           source ${VENV_DIR}/bin/activate
           python onnx-ops/update_config_xfails.py \
             --log-file=/tmp/onnx_ops_cpu_logs.json \
             --config-file=${CONFIG_FILE_PATH}
           cat ${CONFIG_FILE_PATH}
       - name: "Uploading new config file"
-        if: failure()
         uses: actions/upload-artifact@v4
         with:
           name: onnx_ops_cpu_llvm_sync.json

onnx-ops/configs/onnx_ops_cpu_llvm_sync.json

@@ -28,9 +28,6 @@
     "onnx/node/generated/test_affine_grid_3d_expanded",
     "onnx/node/generated/test_ai_onnx_ml_array_feature_extractor",
     "onnx/node/generated/test_ai_onnx_ml_binarizer",
-    "onnx/node/generated/test_ai_onnx_ml_label_encoder_string_int",
-    "onnx/node/generated/test_ai_onnx_ml_label_encoder_string_int_no_default",
-    "onnx/node/generated/test_ai_onnx_ml_label_encoder_tensor_value_only_mapping",
     "onnx/node/generated/test_ai_onnx_ml_tree_ensemble_set_membership",
     "onnx/node/generated/test_ai_onnx_ml_tree_ensemble_single_tree",
     "onnx/node/generated/test_averagepool_2d_precomputed_same_upper",
@@ -62,12 +59,10 @@
     "onnx/node/generated/test_cast_FLOAT_to_FLOAT8E5M2",
     "onnx/node/generated/test_cast_FLOAT_to_FLOAT8E5M2FNUZ",
     "onnx/node/generated/test_cast_FLOAT_to_INT4",
-    "onnx/node/generated/test_cast_FLOAT_to_STRING",
     "onnx/node/generated/test_cast_FLOAT_to_UINT4",
     "onnx/node/generated/test_cast_INT4_to_FLOAT",
     "onnx/node/generated/test_cast_INT4_to_FLOAT16",
     "onnx/node/generated/test_cast_INT4_to_INT8",
-    "onnx/node/generated/test_cast_STRING_to_FLOAT",
     "onnx/node/generated/test_cast_UINT4_to_FLOAT",
     "onnx/node/generated/test_cast_UINT4_to_FLOAT16",
     "onnx/node/generated/test_cast_UINT4_to_UINT8",
@@ -97,10 +92,6 @@
     "onnx/node/generated/test_castlike_FLOAT_to_FLOAT8E5M2FNUZ",
     "onnx/node/generated/test_castlike_FLOAT_to_FLOAT8E5M2FNUZ_expanded",
     "onnx/node/generated/test_castlike_FLOAT_to_FLOAT8E5M2_expanded",
-    "onnx/node/generated/test_castlike_FLOAT_to_STRING",
-    "onnx/node/generated/test_castlike_FLOAT_to_STRING_expanded",
-    "onnx/node/generated/test_castlike_STRING_to_FLOAT",
-    "onnx/node/generated/test_castlike_STRING_to_FLOAT_expanded",
     "onnx/node/generated/test_center_crop_pad_crop_axes_chw_expanded",
     "onnx/node/generated/test_center_crop_pad_crop_axes_hwc_expanded",
     "onnx/node/generated/test_center_crop_pad_crop_negative_axes_hwc_expanded",
@@ -149,8 +140,6 @@
     "onnx/node/generated/test_einsum_batch_matmul",
     "onnx/node/generated/test_einsum_inner_prod",
     "onnx/node/generated/test_einsum_sum",
-    "onnx/node/generated/test_equal_string",
-    "onnx/node/generated/test_equal_string_broadcast",
     "onnx/node/generated/test_gathernd_example_int32_batch_dim1",
     "onnx/node/generated/test_gridsample_bicubic",
     "onnx/node/generated/test_gridsample_bicubic_align_corners_0_additional_1",
@@ -349,9 +338,6 @@
     "onnx/node/generated/test_reduce_sum_square_negative_axes_keepdims_random",
     "onnx/node/generated/test_reduce_sum_square_negative_axes_keepdims_random_expanded",
     "onnx/node/generated/test_reflect_pad",
-    "onnx/node/generated/test_regex_full_match_basic",
-    "onnx/node/generated/test_regex_full_match_email_domain",
-    "onnx/node/generated/test_regex_full_match_empty",
     "onnx/node/generated/test_reshape_allowzero_reordered",
     "onnx/node/generated/test_resize_downsample_scales_cubic",
     "onnx/node/generated/test_resize_downsample_scales_cubic_A_n0p5_exclude_outside",
@@ -451,23 +437,6 @@
     "onnx/node/generated/test_softsign_example",
     "onnx/node/generated/test_stft",
     "onnx/node/generated/test_stft_with_window",
-    "onnx/node/generated/test_string_concat",
-    "onnx/node/generated/test_string_concat_broadcasting",
-    "onnx/node/generated/test_string_concat_empty_string",
-    "onnx/node/generated/test_string_concat_utf8",
-    "onnx/node/generated/test_string_concat_zero_dimensional",
-    "onnx/node/generated/test_string_split_basic",
-    "onnx/node/generated/test_string_split_consecutive_delimiters",
-    "onnx/node/generated/test_string_split_empty_string_delimiter",
-    "onnx/node/generated/test_string_split_empty_tensor",
-    "onnx/node/generated/test_string_split_maxsplit",
-    "onnx/node/generated/test_string_split_no_delimiter",
-    "onnx/node/generated/test_strnormalizer_export_monday_casesensintive_lower",
-    "onnx/node/generated/test_strnormalizer_export_monday_casesensintive_nochangecase",
-    "onnx/node/generated/test_strnormalizer_export_monday_casesensintive_upper",
-    "onnx/node/generated/test_strnormalizer_export_monday_empty_output",
-    "onnx/node/generated/test_strnormalizer_export_monday_insensintive_upper_twodim",
-    "onnx/node/generated/test_strnormalizer_nostopwords_nochangecase",
     "onnx/node/generated/test_tfidfvectorizer_tf_batch_onlybigrams_skip0",
     "onnx/node/generated/test_tfidfvectorizer_tf_batch_onlybigrams_skip5",
     "onnx/node/generated/test_tfidfvectorizer_tf_batch_uniandbigrams_skip5",

onnx-ops/import_onnx_tests.py

@@ -5,15 +5,17 @@
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 import argparse
+import logging
 import onnx
-from multiprocessing import Pool
-from pathlib import Path
-from onnx import version_converter
 import shutil
 import subprocess
 import sys
 from import_onnx_tests_utils import *
+from multiprocessing import Pool
+from onnx import version_converter
+from pathlib import Path
 
+logger = logging.getLogger(__name__)
 
 ONNX_PACKAGE_DIR = Path(onnx.__file__).parent
 ONNX_NODE_TESTS_ROOT = ONNX_PACKAGE_DIR / "backend/test/data/node"
@@ -37,11 +39,14 @@ def find_onnx_tests(root_dir_path: Path):
 def import_onnx_files_with_cleanup(test_dir_path: Path):
     test_name = test_dir_path.name
     imported_dir_path = Path(GENERATED_FILES_OUTPUT_ROOT) / test_name
-    result = import_onnx_files(test_dir_path, imported_dir_path)
-    if not result:
+    try:
+        import_onnx_files(test_dir_path, imported_dir_path)
+    except Exception as e:
+        logger.error(f"Error importing {test_name}: {e}")
         # Note: could comment this out to keep partially imported directories.
         shutil.rmtree(imported_dir_path)
-    return (test_name, result)
+        return (test_name, False)
+    return (test_name, True)
 
 
 def import_onnx_files(test_dir_path: Path, imported_dir_path: Path):
@@ -97,14 +102,19 @@ def import_onnx_files(test_dir_path: Path, imported_dir_path: Path):
     ]
     ret = subprocess.run(exec_args, capture_output=True)
     if ret.returncode != 0:
-        # TODO(scotttodd): log ret.stdout and ret.stderr to a file/folder?
-        print(f"  {imported_dir_path.name[5:]} import failed", file=sys.stderr)
-        return False
+        logger.error(
+            f"Import of {imported_dir_path.name} failed!\niree-import-onnx stdout:"
+        )
+        logger.error(ret.stdout.decode("utf-8"))
+        logger.error("iree-import-onnx stderr:")
+        logger.error(ret.stderr.decode("utf-8"))
+        raise RuntimeError(f"  {imported_dir_path.name} import failed")
 
     test_data_dirs = sorted(test_dir_path.glob("test_data_set*"))
     if len(test_data_dirs) != 1:
-        print("WARNING: unhandled 'len(test_data_dirs) != 1'")
-        return False
+        raise NotImplementedError(
+            f"Unhandled 'len(test_data_dirs) != 1' for {imported_dir_path.name}"
+        )
 
     # Convert from:
     #   * input/output_*.pb
@@ -122,9 +132,6 @@ def import_onnx_files(test_dir_path: Path, imported_dir_path: Path):
         type_proto = model.graph.input[i].type
         converted_data = convert_onnx_proto_to_numpy_array(test_input, type_proto)
         converted_type = convert_onnx_type_proto_to_iree_type_string(type_proto)
-        # TODO(scotttodd): raise exception instead of None as flow control?
-        if converted_data is None or converted_type is None:
-            return False
 
         input_path_bin = (imported_dir_path / test_input.stem).with_suffix(".bin")
         write_binary_to_file(converted_data, input_path_bin)
@@ -137,9 +144,6 @@ def import_onnx_files(test_dir_path: Path, imported_dir_path: Path):
         type_proto = model.graph.output[i].type
         converted_data = convert_onnx_proto_to_numpy_array(test_output, type_proto)
         converted_type = convert_onnx_type_proto_to_iree_type_string(type_proto)
-        # TODO(scotttodd): raise exception instead of None as flow control?
-        if converted_data is None or converted_type is None:
-            return False
 
         output_path_bin = (imported_dir_path / test_output.stem).with_suffix(".bin")
         write_binary_to_file(converted_data, output_path_bin)
@@ -150,8 +154,6 @@ def import_onnx_files(test_dir_path: Path, imported_dir_path: Path):
     with open(test_data_flagfile_path, "wt") as f:
         f.writelines(test_data_flagfile_lines)
 
-    return True
-
 
 if __name__ == "__main__":
     parser = argparse.ArgumentParser(description="ONNX test case importer.")

onnx-ops/import_onnx_tests_utils.py

@@ -4,11 +4,11 @@
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-import struct
 import numpy as np
 import onnx
-from pathlib import Path
+import struct
 from onnx import numpy_helper
+from pathlib import Path
 
 
 def convert_onnx_proto_to_numpy_array(
@@ -21,9 +21,7 @@ def convert_onnx_proto_to_numpy_array(
             tensor.ParseFromString(protobuf_content)
             return numpy_helper.to_array(tensor)
         else:
-            # TODO(scotttodd): raise exception / log to file
-            print(f"Unsupported proto type: {type_proto}")
-            return None
+            raise NotImplementedError(f"Unsupported proto type: {type_proto}")
 
 
 def convert_proto_elem_type_to_iree_dtype(etype):
@@ -84,9 +82,7 @@ def convert_onnx_type_proto_to_iree_type_string(
             return dtype
         return f"{shape}x{dtype}"
     else:
-        # TODO(scotttodd): raise exception / log to file
-        print(f"Unsupported proto type: {type_proto}")
-        return None
+        raise NotImplementedError(f"Unsupported proto type: {type_proto}")
 
 
 # map numpy dtype -> (iree dtype, struct.pack format str)
@@ -114,8 +110,9 @@ def pack_ndarray_to_binary(ndarr: np.ndarray):
         iree_dtype = numpy_to_iree_dtype_map[dtype][1]
         bytearr = struct.pack(f"{len(mylist)}{iree_dtype}", *mylist)
     else:
-        print(f"WARNING: unsupported data type in pack_ndarray_to_binary() : '{dtype}'")
-        # TODO(scotttodd): raise exception / log to file
+        raise NotImplementedError(
+            f"Unsupported data type in pack_ndarray_to_binary(): '{dtype}'"
+        )
     return bytearr
 
 

onnx-ops/onnx/node/generated/import_failures.txt

@@ -1,4 +1,15 @@
+test_ai_onnx_ml_label_encoder_string_int
+test_ai_onnx_ml_label_encoder_string_int_no_default
 test_ai_onnx_ml_label_encoder_tensor_mapping
+test_ai_onnx_ml_label_encoder_tensor_value_only_mapping
+test_cast_FLOAT_to_STRING
+test_cast_STRING_to_FLOAT
+test_castlike_FLOAT_to_STRING
+test_castlike_FLOAT_to_STRING_expanded
+test_castlike_STRING_to_FLOAT
+test_castlike_STRING_to_FLOAT_expanded
+test_equal_string
+test_equal_string_broadcast
 test_identity_opt
 test_identity_sequence
 test_if_opt
@@ -11,6 +22,9 @@ test_optional_get_element_sequence
 test_optional_has_element_empty_optional_input
 test_optional_has_element_optional_input
 test_optional_has_element_tensor_input
+test_regex_full_match_basic
+test_regex_full_match_email_domain
+test_regex_full_match_empty
 test_sequence_insert_at_back
 test_sequence_insert_at_front
 test_sequence_map_add_1_sequence_1_tensor
@@ -27,4 +41,21 @@ test_sequence_map_identity_2_sequences
 test_sequence_map_identity_2_sequences_expanded
 test_split_to_sequence_1
 test_split_to_sequence_2
-test_split_to_sequence_nokeepdims
+test_split_to_sequence_nokeepdims
+test_string_concat
+test_string_concat_broadcasting
+test_string_concat_empty_string
+test_string_concat_utf8
+test_string_concat_zero_dimensional
+test_string_split_basic
+test_string_split_consecutive_delimiters
+test_string_split_empty_string_delimiter
+test_string_split_empty_tensor
+test_string_split_maxsplit
+test_string_split_no_delimiter
+test_strnormalizer_export_monday_casesensintive_lower
+test_strnormalizer_export_monday_casesensintive_nochangecase
+test_strnormalizer_export_monday_casesensintive_upper
+test_strnormalizer_export_monday_empty_output
+test_strnormalizer_export_monday_insensintive_upper_twodim
+test_strnormalizer_nostopwords_nochangecase

onnx-ops/onnx/node/generated/import_successes.txt

@@ -20,9 +20,6 @@ test_affine_grid_3d_align_corners_expanded
 test_affine_grid_3d_expanded
 test_ai_onnx_ml_array_feature_extractor
 test_ai_onnx_ml_binarizer
-test_ai_onnx_ml_label_encoder_string_int
-test_ai_onnx_ml_label_encoder_string_int_no_default
-test_ai_onnx_ml_label_encoder_tensor_value_only_mapping
 test_ai_onnx_ml_tree_ensemble_set_membership
 test_ai_onnx_ml_tree_ensemble_single_tree
 test_and2d
@@ -160,12 +157,10 @@ test_cast_FLOAT_to_FLOAT8E4M3FNUZ
 test_cast_FLOAT_to_FLOAT8E5M2
 test_cast_FLOAT_to_FLOAT8E5M2FNUZ
 test_cast_FLOAT_to_INT4
-test_cast_FLOAT_to_STRING
 test_cast_FLOAT_to_UINT4
 test_cast_INT4_to_FLOAT
 test_cast_INT4_to_FLOAT16
 test_cast_INT4_to_INT8
-test_cast_STRING_to_FLOAT
 test_cast_UINT4_to_FLOAT
 test_cast_UINT4_to_FLOAT16
 test_cast_UINT4_to_UINT8
@@ -209,10 +204,6 @@ test_castlike_FLOAT_to_FLOAT8E5M2
 test_castlike_FLOAT_to_FLOAT8E5M2FNUZ
 test_castlike_FLOAT_to_FLOAT8E5M2FNUZ_expanded
 test_castlike_FLOAT_to_FLOAT8E5M2_expanded
-test_castlike_FLOAT_to_STRING
-test_castlike_FLOAT_to_STRING_expanded
-test_castlike_STRING_to_FLOAT
-test_castlike_STRING_to_FLOAT_expanded
 test_ceil
 test_ceil_example
 test_celu
@@ -358,8 +349,6 @@ test_elu_example_expanded_ver18
 test_elu_expanded_ver18
 test_equal
 test_equal_bcast
-test_equal_string
-test_equal_string_broadcast
 test_erf
 test_exp
 test_exp_example
@@ -914,9 +903,6 @@ test_reduce_sum_square_negative_axes_keepdims_example_expanded
 test_reduce_sum_square_negative_axes_keepdims_random
 test_reduce_sum_square_negative_axes_keepdims_random_expanded
 test_reflect_pad
-test_regex_full_match_basic
-test_regex_full_match_email_domain
-test_regex_full_match_empty
 test_relu
 test_relu_expanded_ver18
 test_reshape_allowzero_reordered
@@ -1149,23 +1135,6 @@ test_squeeze
 test_squeeze_negative_axes
 test_stft
 test_stft_with_window
-test_string_concat
-test_string_concat_broadcasting
-test_string_concat_empty_string
-test_string_concat_utf8
-test_string_concat_zero_dimensional
-test_string_split_basic
-test_string_split_consecutive_delimiters
-test_string_split_empty_string_delimiter
-test_string_split_empty_tensor
-test_string_split_maxsplit
-test_string_split_no_delimiter
-test_strnormalizer_export_monday_casesensintive_lower
-test_strnormalizer_export_monday_casesensintive_nochangecase
-test_strnormalizer_export_monday_casesensintive_upper
-test_strnormalizer_export_monday_empty_output
-test_strnormalizer_export_monday_insensintive_upper_twodim
-test_strnormalizer_nostopwords_nochangecase
 test_sub
 test_sub_bcast
 test_sub_example