From c7cde6a102a98f1e66d06f064915eaccd97b7c99 Mon Sep 17 00:00:00 2001 From: Anastasiia Pnevskaia Date: Fri, 28 Jul 2023 13:46:47 +0200 Subject: [PATCH] Removed saved_model_tags from OVC, improved help. (#18785) * Removed saved_model_tags, improved help. * Fixed help, removed to string conversion of params. * Help corrections. * Small correction. * Corrected wrong changes. * Corrected wrong changes. * Fixed arguments creating in cli parser. * Renamed extensions parameter to extension. --- .../test_mo_convert_extensions.py | 4 +- .../mo_python_api_tests/test_mo_convert_tf.py | 2 + tools/ovc/openvino/tools/ovc/cli_parser.py | 329 ++++-------------- tools/ovc/openvino/tools/ovc/convert.py | 12 +- tools/ovc/openvino/tools/ovc/convert_impl.py | 83 ++--- tools/ovc/openvino/tools/ovc/help.py | 22 +- .../tools/ovc/moc_frontend/check_config.py | 16 +- .../tools/ovc/moc_frontend/pipeline.py | 3 - .../moc_tf_fe/check_info_messages_test.py | 2 +- tools/ovc/unit_tests/moc_tf_fe/utils.py | 2 +- .../unit_tests/ovc/convert/meta_data_test.py | 2 +- .../ovc/convert/meta_data_test_actual.py | 2 +- .../ovc/utils/args_to_string_test.py | 20 -- .../unit_tests/ovc/utils/cli_parser_test.py | 36 +- 14 files changed, 130 insertions(+), 405 deletions(-) delete mode 100644 tools/ovc/unit_tests/ovc/utils/args_to_string_test.py diff --git a/tests/layer_tests/mo_python_api_tests/test_mo_convert_extensions.py b/tests/layer_tests/mo_python_api_tests/test_mo_convert_extensions.py index fd926c11b3b9e9..5cc02f76f54416 100644 --- a/tests/layer_tests/mo_python_api_tests/test_mo_convert_extensions.py +++ b/tests/layer_tests/mo_python_api_tests/test_mo_convert_extensions.py @@ -102,9 +102,9 @@ def create_ref_graph2(): return Model([sigmoid], [param], "test") test_data = [ - {'params_test': {'extensions': create_custom_extension_leaky_relu_to_relu()}, + {'params_test': {'extension': create_custom_extension_leaky_relu_to_relu()}, 'ref_graph': create_ref_graph1()}, - {'params_test': {'extensions': [create_custom_extension_leaky_relu_to_relu(), + {'params_test': {'extension': [create_custom_extension_leaky_relu_to_relu(), create_custom_extension_elu_to_sigmoid()]}, 'ref_graph': create_ref_graph2()} ] diff --git a/tests/layer_tests/mo_python_api_tests/test_mo_convert_tf.py b/tests/layer_tests/mo_python_api_tests/test_mo_convert_tf.py index 7a4e33c241023b..d493808a7ed8de 100644 --- a/tests/layer_tests/mo_python_api_tests/test_mo_convert_tf.py +++ b/tests/layer_tests/mo_python_api_tests/test_mo_convert_tf.py @@ -756,6 +756,7 @@ def __call__(self, input): arr[2] = 2 # Check model inference + cmp_model = compile_model(ov_model) ov_infer2 = cmp_model(test_input) fw_infer2 = keras_model(test_input).numpy() @@ -856,6 +857,7 @@ def test_memory_loss(self, ie_device, precision, ir_version, temp_dir): gc.collect() # Check model inference + cmp_model = compile_model(ov_model) ov_infer2 = cmp_model(test_input, ie_device) feed_dict = {"Input:0": test_input} diff --git a/tools/ovc/openvino/tools/ovc/cli_parser.py b/tools/ovc/openvino/tools/ovc/cli_parser.py index e4b7904e2f1568..7ae175df1322e9 100644 --- a/tools/ovc/openvino/tools/ovc/cli_parser.py +++ b/tools/ovc/openvino/tools/ovc/cli_parser.py @@ -5,6 +5,7 @@ import ast import logging as log import os +import pathlib import re from collections import OrderedDict, namedtuple from distutils.util import strtobool @@ -22,84 +23,7 @@ from openvino.tools.ovc.convert_data_type import destination_type_to_np_data_type from openvino.tools.ovc.error import Error from openvino.tools.ovc.utils import get_mo_root_dir -from openvino.tools.ovc.help import get_convert_model_help_specifics, get_to_string_methods_for_params - - -def extension_path_to_str_or_extensions_class(extension): - if isinstance(extension, str): - return extension - elif isinstance(extension, Path): - return str(extension) - else: - # Return unknown object as is. - # The type of the object will be checked by frontend.add_extension() method - return extension - - -def extensions_to_str_or_extensions_class(extensions): - if extensions is None: - return None - extensions_list = [] - if isinstance(extensions, str): - extensions_list = extensions.split(',') - elif isinstance(extensions, list): - for ext in extensions: - ext = extension_path_to_str_or_extensions_class(ext) - extensions_list.append(ext) - else: - extensions_list = [extension_path_to_str_or_extensions_class(extensions)] - - for ext in extensions_list: - if isinstance(ext, str): - readable_file_or_dir(ext) - return extensions_list - - -def path_to_str(path): - if path is None: - return None - if isinstance(path, str): - return path - elif isinstance(path, Path): - return str(path) - else: - raise Exception("Incorrect type of {} expected str or Path, got {}".format(path, type(path))) - - -def path_to_str_or_object(value): - if value is None or isinstance(value, str): - return value - elif isinstance(value, Path): - return str(value) - else: - return value - - -def paths_to_str(paths): - if paths is None: - return None - if isinstance(paths, list): - paths_str = [] - for path in paths: - paths_str.append(path_to_str(path)) - return ','.join(paths_str) - else: - path_to_str(paths) - - -def str_list_to_str(values): - if values is None: - return None - if isinstance(values, str): - return values - elif isinstance(values, list): - for value in values: - if not isinstance(value, str): - raise Error("Incorrect argument. {} expected to string, got type {}.".format(value, type(value))) - return ','.join(values) - else: - raise Error("Incorrect argument. {} expected to string or list of strings, got type {}.".format(values, type(values))) - +from openvino.tools.ovc.help import get_convert_model_help_specifics def is_shape_type(value): if isinstance(value, PartialShape): @@ -113,25 +37,6 @@ def is_shape_type(value): return True return False - -def value_to_str(value, separator): - if isinstance(value, np.ndarray): - values = [] - for x in np.nditer(value): - values.append(str(x)) - return "[" + separator.join(values) + "]" - if isinstance(value, list): - values = [] - for x in value: - if not isinstance(x, numbers.Number): - raise Exception("Incorrect value type. Expected numeric value, got {}".format(type(x))) - values.append(str(x)) - return "[" + separator.join(values) + "]" - if isinstance(value, bool): - return "True" if value else "False" - raise Exception("Incorrect value type. Expected np.ndarray or list, got {}".format(type(value))) - - def single_input_to_input_cut_info(input: [str, tuple, list, PartialShape, Type, type]): """ Parses parameters of single input to InputCutInfo. @@ -270,23 +175,13 @@ def freeze_placeholder_to_input_cut_info(inputs: list): return placeholder_values, unnamed_placeholder_values - -def layout_to_str(layout): - if isinstance(layout, str): - return layout - if isinstance(layout, Layout): - return layout.to_string() - raise Exception("Incorrect layout type. Expected Layout or string or dictionary, " - "where key is operation name and value is layout or list of layouts, got {}".format(type(layout))) - -ParamDescription = namedtuple("ParamData", - ["description", "cli_tool_description", "to_string"]) +ParamDescription = namedtuple("ParamData", ["description", "cli_tool_description"]) def get_mo_convert_params(): mo_convert_docs = openvino.tools.ovc.convert_model.__doc__ # pylint: disable=no-member mo_convert_params = {} - group = "Framework-agnostic parameters:" #FIXME: WA for unknown bug in this function + group = "Optional parameters:" #FIXME: WA for unknown bug in this function mo_convert_params[group] = {} mo_convert_docs = mo_convert_docs[:mo_convert_docs.find('Returns:')] @@ -309,7 +204,7 @@ def get_mo_convert_params(): param_description = param_description[:group_name_idx] param_description = param_description.strip() - mo_convert_params[group][param_name] = ParamDescription(param_description, "", None) + mo_convert_params[group][param_name] = ParamDescription(param_description, "") mo_convert_docs = mo_convert_docs[param_description_idx:] @@ -317,59 +212,21 @@ def get_mo_convert_params(): mo_convert_params[group_name] = {} group = group_name - # TODO: remove this when internal converting of params to string is removed <-- DO IT - params_converted_to_string = get_to_string_methods_for_params() - - params_with_paths = get_params_with_paths_list() cli_tool_specific_descriptions = get_convert_model_help_specifics() for group_name, param_group in mo_convert_params.items(): for param_name, d in param_group.items(): - to_str_method = None - if param_name in params_converted_to_string: - to_str_method = params_converted_to_string[param_name] - elif param_name in params_with_paths: - to_str_method = path_to_str - cli_tool_description = None if param_name in cli_tool_specific_descriptions: cli_tool_description = cli_tool_specific_descriptions[param_name] desc = ParamDescription(d.description, - cli_tool_description, - to_str_method) + cli_tool_description) mo_convert_params[group_name][param_name] = desc return mo_convert_params -class DeprecatedStoreTrue(argparse.Action): - def __init__(self, nargs=0, **kw): - super().__init__(nargs=nargs, **kw) - - def __call__(self, parser, namespace, values, option_string=None): - dep_msg = "Use of deprecated cli option {} detected. Option use in the following releases will be fatal. ".format(option_string) - log.error(dep_msg, extra={'is_warning': True}) - setattr(namespace, self.dest, True) - - -class DeprecatedOptionCommon(argparse.Action): - def __call__(self, parser, args, values, option_string): - dep_msg = "Use of deprecated cli option {} detected. Option use in the following releases will be fatal. ".format(option_string) - log.error(dep_msg, extra={'is_warning': True}) - setattr(args, self.dest, values) - - -class IgnoredAction(argparse.Action): - def __init__(self, nargs=0, **kw): - super().__init__(nargs=nargs, **kw) - - def __call__(self, parser, namespace, values, option_string=None): - dep_msg = "Use of removed cli option '{}' detected. The option is ignored. ".format(option_string) - log.error(dep_msg, extra={'is_warning': True}) - setattr(namespace, self.dest, True) - - def canonicalize_and_check_paths(values: Union[str, List[str], None], param_name, try_mo_root=False, check_existence=True) -> List[str]: if values is not None: @@ -380,15 +237,18 @@ def canonicalize_and_check_paths(values: Union[str, List[str], None], param_name elif isinstance(values, list): list_of_values = values else: - raise Error('Unsupported type of command line parameter "{}" value'.format(param_name)) + return values if not check_existence: return [get_absolute_path(path) for path in list_of_values] for idx, val in enumerate(list_of_values): + if not isinstance(val, (str, pathlib.Path)): + continue + list_of_values[idx] = val - error_msg = 'The value for command line parameter "{}" must be existing file/directory, ' \ + error_msg = 'The value for parameter "{}" must be existing file/directory, ' \ 'but "{}" does not exist.'.format(param_name, val) if os.path.exists(val): continue @@ -403,34 +263,12 @@ def canonicalize_and_check_paths(values: Union[str, List[str], None], param_name return [get_absolute_path(path) for path in list_of_values] -class CanonicalizePathAction(argparse.Action): - """ - Expand user home directory paths and convert relative-paths to absolute. - """ - - def __call__(self, parser, namespace, values, option_string=None): - list_of_paths = canonicalize_and_check_paths(values, param_name=option_string, - try_mo_root=False, check_existence=False) - setattr(namespace, self.dest, list_of_paths) - - -class CanonicalizeTransformationPathCheckExistenceAction(argparse.Action): - """ - Convert relative to the current and relative to mo root paths to absolute - and check specified file or directory existence. - """ - - def __call__(self, parser, namespace, values, option_string=None): - list_of_paths = canonicalize_and_check_paths(values, param_name=option_string, - try_mo_root=True, check_existence=True) - setattr(namespace, self.dest, ','.join(list_of_paths)) - - class CanonicalizePathCheckExistenceAction(argparse.Action): """ Expand user home directory paths and convert relative-paths to absolute and check specified file or directory existence. """ + check_value = canonicalize_and_check_paths def __call__(self, parser, namespace, values, option_string=None): list_of_paths = canonicalize_and_check_paths(values, param_name=option_string, @@ -438,44 +276,14 @@ def __call__(self, parser, namespace, values, option_string=None): setattr(namespace, self.dest, list_of_paths) -class CanonicalizeExtensionsPathCheckExistenceAction(argparse.Action): - """ - Expand user home directory paths and convert relative-paths to absolute and check specified file or directory - existence. - """ - - def __call__(self, parser, namespace, values, option_string=None): - list_of_paths = canonicalize_and_check_paths(values, param_name=option_string, - try_mo_root=False, check_existence=True) - # Extensions paths are needed to be stored as list - setattr(namespace, self.dest, list_of_paths) - - -class CanonicalizePathCheckExistenceIfNeededAction(CanonicalizePathCheckExistenceAction): - - def __call__(self, parser, namespace, values, option_string=None): - if values is not None: - if isinstance(values, str): - if values != "": - super().__call__(parser, namespace, values, option_string) - else: - setattr(namespace, self.dest, values) - - -class DeprecatedCanonicalizePathCheckExistenceAction(CanonicalizePathCheckExistenceAction): - def __call__(self, parser, namespace, values, option_string=None): - dep_msg = "Use of deprecated cli option {} detected. Option use in the following releases will be fatal. ".format( - option_string) - log.error(dep_msg, extra={'is_warning': True}) - super().__call__(parser, namespace, values, option_string) - - -def readable_file(path: str): +def readable_file_or_object(path: str): """ Check that specified path is a readable file. :param path: path to check :return: path if the file is readable """ + if not isinstance(path, (str, pathlib.Path)): + return path if not os.path.isfile(path): raise Error('The "{}" is not existing file'.format(path)) elif not os.access(path, os.R_OK): @@ -484,12 +292,14 @@ def readable_file(path: str): return path -def readable_file_or_dir(path: str): +def readable_file_or_dir_or_object(path: str): """ Check that specified path is a readable file or directory. :param path: path to check :return: path if the file/directory is readable """ + if not isinstance(path, (str, pathlib.Path)): + return path if not os.path.isfile(path) and not os.path.isdir(path): raise Error('The "{}" is not existing file or directory'.format(path)) elif not os.access(path, os.R_OK): @@ -498,36 +308,31 @@ def readable_file_or_dir(path: str): return path -def readable_dirs(paths: str): - """ - Checks that comma separated list of paths are readable directories. - :param paths: comma separated list of paths. - :return: comma separated list of paths. +def readable_dirs_or_files_or_empty(paths: [str, list, tuple]): """ - paths_list = [readable_dir(path) for path in paths.split(',')] - return ','.join(paths_list) - - -def readable_dirs_or_empty(paths: str): - """ - Checks that comma separated list of paths are readable directories of if it is empty. + Checks that comma separated list of paths are readable directories, files or a provided path is empty. :param paths: comma separated list of paths. :return: comma separated list of paths. """ - if paths: - return readable_dirs(paths) - return paths + paths_list = paths + if isinstance(paths, (list, tuple)): + paths_list = [readable_file_or_dir_or_object(path) for path in paths] + if isinstance(paths, (str, pathlib.Path)): + paths_list = [readable_file_or_dir_or_object(path) for path in paths.split(',')] + return paths_list[0] if isinstance(paths, (list, tuple)) and len(paths_list) == 1 else paths_list -def readable_dirs_or_files_or_empty(paths: str): +def readable_files_or_empty(paths: [str, list, tuple]): """ Checks that comma separated list of paths are readable directories, files or a provided path is empty. :param paths: comma separated list of paths. :return: comma separated list of paths. """ - if paths: - paths_list = [readable_file_or_dir(path) for path in paths.split(',')] - return ','.join(paths_list) + if isinstance(paths, (list, tuple)): + return [readable_file_or_object(path) for path in paths] + if isinstance(paths, (str, pathlib.Path)): + paths_list = [readable_file_or_object(path) for path in paths.split(',')] + return paths_list return paths @@ -612,10 +417,10 @@ def add_args_by_description(args_group, params_description): args_group.add_argument( cli_param_name, *param_alias, type=str if param_type is None else param_type, - nargs='*' if param_name == 'input_model' else '?', action=action, help=help_text, - default=None if param_name == 'input_model' else signature.parameters[param_name].default) + default=None if param_name == 'input_model' else signature.parameters[param_name].default, + metavar=param_name.upper() if param_name == 'input_model' else None) # Other params else: additional_params = {} @@ -633,25 +438,39 @@ def add_args_by_description(args_group, params_description): **additional_params ) +class Formatter(argparse.HelpFormatter): + def _format_usage(self, usage, actions, groups, prefix): + usage = argparse.HelpFormatter._format_usage(self, usage, actions, groups, prefix) + usage = usage[0:usage.find('INPUT_MODEL')].rstrip() + '\n' + insert_idx = usage.find(self._prog) + len(self._prog) + usage = usage[0: insert_idx] + ' INPUT_MODEL ' + usage[insert_idx + 1:] + return usage + + def _get_default_metavar_for_optional(self, action): + if action.option_strings == ['--compress_to_fp16']: + return "True | False" + return argparse.HelpFormatter._get_default_metavar_for_optional(self, action) + def get_common_cli_parser(parser: argparse.ArgumentParser = None): if not parser: - parser = argparse.ArgumentParser() - common_group = parser.add_argument_group('Framework-agnostic parameters') + parser = argparse.ArgumentParser(formatter_class=Formatter) mo_convert_params = get_mo_convert_params() - mo_convert_params_common = mo_convert_params['Framework-agnostic parameters:'] + mo_convert_params_common = mo_convert_params['Optional parameters:'] from openvino.tools.ovc.version import VersionChecker # Command line tool specific params - common_group.add_argument('--output_model', + parser.add_argument('--output_model', help='This parameter is used to name output .xml/.bin files with converted model.') - common_group.add_argument('--compress_to_fp16', type=check_bool, default=True, - help='Compress weights in output IR .xml/bin files to FP16.') - common_group.add_argument('--version', action='version', + parser.add_argument('--compress_to_fp16', type=check_bool, default=True, nargs='?', + help='Compress weights in output OpenVINO model to FP16. ' + 'To turn off compression use "--compress_to_fp16=False" command line parameter. ' + 'Default value is True.') + parser.add_argument('--version', action='version', help='Print ovc version and exit.', version='OpenVINO Model Converter (ovc) {}'.format(VersionChecker().get_ie_version())) - add_args_by_description(common_group, mo_convert_params_common) + add_args_by_description(parser, mo_convert_params_common) return parser @@ -667,33 +486,7 @@ def get_common_cli_options(model_name): def get_params_with_paths_list(): - return ['input_model', 'output_model', 'extensions'] - - -def get_tf_cli_parser(parser: argparse.ArgumentParser = None): - """ - Specifies cli arguments for Model Conversion for TF - - Returns - ------- - ArgumentParser instance - """ - mo_convert_params_tf = get_mo_convert_params()['TensorFlow*-specific parameters:'] - - tf_group = parser.add_argument_group('TensorFlow*-specific parameters') - add_args_by_description(tf_group, mo_convert_params_tf) - return parser - - -def get_onnx_cli_parser(parser: argparse.ArgumentParser = None): - """ - Specifies cli arguments for Model Conversion for ONNX - - Returns - ------- - ArgumentParser instance - """ - return parser + return ['input_model', 'output_model', 'extension'] def get_all_cli_parser(): @@ -704,11 +497,9 @@ def get_all_cli_parser(): ------- ArgumentParser instance """ - parser = argparse.ArgumentParser() + parser = argparse.ArgumentParser(formatter_class=Formatter) get_common_cli_parser(parser=parser) - get_tf_cli_parser(parser=parser) - get_onnx_cli_parser(parser=parser) return parser @@ -1169,6 +960,8 @@ def get_absolute_path(path_to_file: str) -> str: Returns: absolute path of the file """ + if not isinstance(path_to_file, (str, pathlib.Path)): + return path_to_file file_path = os.path.expanduser(path_to_file) if not os.path.isabs(file_path): file_path = os.path.join(os.getcwd(), file_path) @@ -1240,7 +1033,7 @@ def check_bool(value): def depersonalize(value: str, key: str): dir_keys = [ - 'output_dir', 'extensions', 'saved_model_dir', 'tensorboard_logdir', 'caffe_parser_path' + 'output_dir', 'extension', 'saved_model_dir', 'tensorboard_logdir', 'caffe_parser_path' ] if isinstance(value, list): updated_value = [] diff --git a/tools/ovc/openvino/tools/ovc/convert.py b/tools/ovc/openvino/tools/ovc/convert.py index 6aaa0e8d7f7883..ebeb37abd04b4b 100644 --- a/tools/ovc/openvino/tools/ovc/convert.py +++ b/tools/ovc/openvino/tools/ovc/convert.py @@ -23,15 +23,12 @@ def convert_model( input: [str, list, tuple, InputCutInfo] = None, output: [str, list] = None, example_input: Any = None, - extensions: [str, pathlib.Path, list, Any] = None, + extension: [str, pathlib.Path, list, Any] = None, verbose: bool = False, share_weights: bool = True, # PaddlePaddle-specific parameters: example_output: Any = None, # TODO: Consider removing - - # TensorFlow*-specific parameters - saved_model_tags: [str, list] = None, # TODO: Consider removing ) -> Model: """ Converts the model from original framework to OpenVino Model. @@ -96,7 +93,7 @@ def convert_model( For PyTorch it can be torch.Tensor. For Tensorflow it can be tf.Tensor or numpy.ndarray. For PaddlePaddle it can be Paddle Variable. - :param extensions: + :param extension: Paths to libraries (.so or .dll) with extensions, comma-separated list of paths, objects derived from BaseExtension class or lists of objects. For the legacy MO path (if "use_legacy_frontend" is used), @@ -115,11 +112,6 @@ def convert_model( :param example_output: Sample of model output in original framework. For PaddlePaddle it can be Paddle Variable. - TensorFlow*-specific parameters: - :param saved_model_tags: - Group of tag(s) of the MetaGraphDef to load, in string format, separated - by ','. For tag-set contains multiple tags, all tags must be passed in. - Returns: openvino.runtime.Model """ diff --git a/tools/ovc/openvino/tools/ovc/convert_impl.py b/tools/ovc/openvino/tools/ovc/convert_impl.py index cf67c1c078de07..68228093654801 100644 --- a/tools/ovc/openvino/tools/ovc/convert_impl.py +++ b/tools/ovc/openvino/tools/ovc/convert_impl.py @@ -5,6 +5,7 @@ import datetime import logging as log import os +import pathlib import sys import traceback from collections import OrderedDict @@ -23,6 +24,7 @@ from openvino.tools.ovc.cli_parser import get_available_front_ends, \ get_common_cli_options, get_model_name_from_args, depersonalize, get_mo_convert_params, \ input_to_input_cut_info, freeze_placeholder_to_input_cut_info +from openvino.tools.ovc.help import get_convert_model_help_specifics from openvino.tools.ovc.error import Error, FrameworkError from openvino.tools.ovc.get_ov_update_message import get_ov_update_message, get_compression_message @@ -84,7 +86,7 @@ def arguments_post_parsing(argv: argparse.Namespace): print_argv(argv, argv.output_model) params_parsing(argv) - argv.output = argv.output.split(',') if argv.output else None + argv.output = argv.output.split(',') if isinstance(argv.output, (str, pathlib.Path)) else argv.output log.debug("Placeholder shapes : {}".format(argv.placeholder_shapes)) @@ -141,7 +143,7 @@ def prepare_ir(argv: argparse.Namespace): t.send_event("mo", "conversion_method", moc_front_end.get_name() + "_frontend") moc_front_end.add_extension(TelemetryExtension("mo", t.send_event, t.send_error, t.send_stack_trace)) if new_extensions_used(argv): - for extension in argv.extensions: + for extension in argv.extension: moc_front_end.add_extension(extension) ov_model = moc_pipeline(argv, moc_front_end) return ov_model @@ -213,31 +215,6 @@ def driver(argv: argparse.Namespace, non_default_params: dict): return ov_model - -def args_dict_to_list(cli_parser, **kwargs): - # This method is needed to prepare args from convert_model() for args_parse(). - # The method will not be needed when cli_parser checks are moved from cli_parser to a separate pass. - import inspect - from openvino.tools.ovc import convert_model - signature = inspect.signature(convert_model) - result = [] - for key, value in kwargs.items(): - if value is None: - continue - if key in signature.parameters and check_values_equal(signature.parameters[key].default, value): - continue - if check_values_equal(cli_parser.get_default(key), value): - continue - # skip parser checking for non str objects - if not isinstance(value, (str, bool)): - continue - result.append('--{}'.format(key)) - if not isinstance(value, bool): - result.append(value) - - return result - - def get_non_default_params(argv, cli_parser): import numbers import inspect @@ -259,19 +236,6 @@ def get_non_default_params(argv, cli_parser): return non_default_params -def params_to_string(**kwargs): - all_params = {} - for key, value in get_mo_convert_params().items(): - all_params.update(value) - - for key, value in kwargs.items(): - if key in all_params: - param_data = all_params[key] - if param_data.to_string is not None: - kwargs[key] = param_data.to_string(value) - return kwargs - - def add_line_breaks(text: str, char_num: int, line_break: str): words = text.replace('\n', "\n ").split(" ") cnt = 0 @@ -393,10 +357,30 @@ def params_parsing(argv: argparse.Namespace): extract_input_info_from_example(argv, inputs) -def pack_params_to_args_namespace(args: dict, cli_parser: argparse.ArgumentParser): - if len(args) > 0: - args_string = params_to_string(**args) - argv, _ = cli_parser.parse_known_args(args_dict_to_list(cli_parser, **args_string)) # FIXME: input_model_args can be a list +def args_to_argv(**kwargs): + argv = argparse.Namespace() + args_specifics = get_convert_model_help_specifics() + + import inspect + from openvino.tools.ovc import convert_model + signature = inspect.signature(convert_model) + for key, value in kwargs.items(): + if value is None and key in signature.parameters: + setattr(argv, key, signature.parameters[key].default) + continue + if key in args_specifics: + param_specifics = args_specifics[key] + if 'action' in param_specifics and hasattr(param_specifics['action'], 'check_value'): + value = param_specifics['action'].check_value(value, key) + if 'type' in param_specifics: + value = param_specifics['type'](value) + setattr(argv, key, value) + return argv + + +def pack_params_to_args_namespace(args: dict, cli_parser: argparse.ArgumentParser, python_api_used): + if python_api_used: + argv = args_to_argv(**args) # get list of all available params for convert_model() all_params = {} @@ -404,14 +388,9 @@ def pack_params_to_args_namespace(args: dict, cli_parser: argparse.ArgumentParse all_params.update(value) # check that there are no unknown params provided - for key, value in args_string.items(): - if key not in argv and key not in all_params.keys(): + for key, value in args.items(): + if key not in all_params.keys(): raise Error("Unrecognized argument: {}".format(key)) - - # Non string params like input_model or extensions are ignored by parse_args() - # so we need to set them in argv separately - if value is not None and not check_values_equal(getattr(argv, key, None), value): - setattr(argv, key, value) else: argv = cli_parser.parse_args() return argv @@ -469,7 +448,7 @@ def _convert(cli_parser: argparse.ArgumentParser, args, python_api_used): pdmodel = paddle_runtime_converter.convert_paddle_to_pdmodel() args['input_model'] = pdmodel - argv = pack_params_to_args_namespace(args, cli_parser) + argv = pack_params_to_args_namespace(args, cli_parser, python_api_used) argv.framework = model_framework argv.is_python_object = inp_model_is_object diff --git a/tools/ovc/openvino/tools/ovc/help.py b/tools/ovc/openvino/tools/ovc/help.py index 6ea8b27c7dd813..1921fddf94b407 100644 --- a/tools/ovc/openvino/tools/ovc/help.py +++ b/tools/ovc/openvino/tools/ovc/help.py @@ -2,8 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 def get_convert_model_help_specifics(): - from openvino.tools.ovc.cli_parser import CanonicalizePathCheckExistenceAction, CanonicalizeExtensionsPathCheckExistenceAction, \ - readable_file_or_dir, readable_dirs_or_files_or_empty + from openvino.tools.ovc.cli_parser import CanonicalizePathCheckExistenceAction, readable_dirs_or_files_or_empty, readable_files_or_empty from openvino.tools.ovc.version import VersionChecker return { 'input_model': @@ -12,7 +11,7 @@ def get_convert_model_help_specifics(): 'Use openvino.convert_model in Python to convert models from Pytorch.' '', 'action': CanonicalizePathCheckExistenceAction, - 'type': readable_file_or_dir, + 'type': readable_dirs_or_files_or_empty, 'aliases': {}}, 'input_shape': {'description': @@ -73,7 +72,7 @@ def get_convert_model_help_specifics(): 'transform "MakeStateful[param_res_names= {\'input_name_1\':' '\'output_name_1\',\'input_name_2\':\'output_name_2\'}]\" \n' 'Available transformations: "LowLatency2", "MakeStateful", "Pruning"'}, - 'extensions': + 'extension': {'description': 'Paths or a comma-separated list of paths to libraries ' '(.so or .dll) with extensions. For the legacy MO path ' @@ -81,21 +80,10 @@ def get_convert_model_help_specifics(): 'comma-separated list of directories with extensions ' 'are supported. To disable all extensions including ' 'those that are placed at the default location, pass an empty string.', - 'action': CanonicalizeExtensionsPathCheckExistenceAction, - 'type': readable_dirs_or_files_or_empty}, + 'action': CanonicalizePathCheckExistenceAction, + 'type': readable_files_or_empty}, 'version': {'action': 'version', #FIXME: Why the following is not accessible from arg parser? 'version': 'OpenVINO Model Converter (ovc) {}'.format(VersionChecker().get_ie_version())}, } - - -# TODO: remove this when internal converting of params to string is removed <-- DO IT -def get_to_string_methods_for_params(): - from openvino.tools.ovc.cli_parser import path_to_str_or_object, str_list_to_str, extensions_to_str_or_extensions_class - return { - 'input_model': path_to_str_or_object, - 'output': str_list_to_str, - 'extensions': extensions_to_str_or_extensions_class, - 'saved_model_tags': str_list_to_str - } diff --git a/tools/ovc/openvino/tools/ovc/moc_frontend/check_config.py b/tools/ovc/openvino/tools/ovc/moc_frontend/check_config.py index 1905d7685477af..77a27870e31f7b 100644 --- a/tools/ovc/openvino/tools/ovc/moc_frontend/check_config.py +++ b/tools/ovc/openvino/tools/ovc/moc_frontend/check_config.py @@ -17,13 +17,15 @@ def any_extensions_used(argv: argparse.Namespace): # Checks that extensions are provided. # Allowed types are string containing path to legacy extension directory # or path to new extension .so file, or classes inherited from BaseExtension. - if not hasattr(argv, 'extensions') or argv.extensions is None: + if not hasattr(argv, 'extension') or argv.extension is None: return False + if not isinstance(argv.extension, (list, tuple)): + argv.extension = [argv.extension] - if isinstance(argv.extensions, list) and len(argv.extensions) > 0: + if isinstance(argv.extension, (list, tuple)) and len(argv.extension) > 0: has_non_default_path = False has_non_str_objects = False - for ext in argv.extensions: + for ext in argv.extension: if not isinstance(ext, str): has_non_str_objects = True continue @@ -33,12 +35,12 @@ def any_extensions_used(argv: argparse.Namespace): return has_non_default_path or has_non_str_objects - raise Exception("Expected list of extensions, got {}.".format(type(argv.extensions))) + raise Exception("Expected list of extensions, got {}.".format(type(argv.extension))) def legacy_extensions_used(argv: argparse.Namespace): if any_extensions_used(argv): - extensions = argv.extensions + extensions = argv.extension legacy_ext_counter = 0 for extension in extensions: if not isinstance(extension, str): @@ -58,8 +60,8 @@ def legacy_extensions_used(argv: argparse.Namespace): def new_extensions_used(argv: argparse.Namespace): if any_extensions_used(argv): - extensions = argv.extensions - if not isinstance(extensions, list): + extensions = argv.extension + if not isinstance(extensions, (list, tuple)): extensions = [extensions] new_ext_counter = 0 for extension in extensions: diff --git a/tools/ovc/openvino/tools/ovc/moc_frontend/pipeline.py b/tools/ovc/openvino/tools/ovc/moc_frontend/pipeline.py index 78c270f1e9c489..a364239b182c39 100644 --- a/tools/ovc/openvino/tools/ovc/moc_frontend/pipeline.py +++ b/tools/ovc/openvino/tools/ovc/moc_frontend/pipeline.py @@ -49,10 +49,7 @@ def moc_pipeline(argv: argparse.Namespace, moc_front_end: FrontEnd): share_weights = getattr(argv, 'share_weights', True) #FIXME: Should be controlled by default value if isinstance(argv.input_model, (tuple, list)) and len(argv.input_model) == 2: # frozen format with v1 checkpoints - assert not hasattr(argv, 'saved_model_tags') or not argv.saved_model_tags input_model = moc_front_end.load([part for part in argv.input_model], share_weights) - elif hasattr(argv, 'saved_model_tags') and argv.saved_model_tags: - input_model = moc_front_end.load([argv.input_model, argv.saved_model_tags], share_weights) else: input_model = moc_front_end.load(argv.input_model, share_weights) diff --git a/tools/ovc/unit_tests/moc_tf_fe/check_info_messages_test.py b/tools/ovc/unit_tests/moc_tf_fe/check_info_messages_test.py index 9508a3a7fd2d6e..87232ed7bd7c18 100644 --- a/tools/ovc/unit_tests/moc_tf_fe/check_info_messages_test.py +++ b/tools/ovc/unit_tests/moc_tf_fe/check_info_messages_test.py @@ -31,7 +31,7 @@ def arg_parse_helper(input_model, input=None, output_dir='.', compress_to_fp16=compress_to_fp16, - extensions=None + extension=None ) diff --git a/tools/ovc/unit_tests/moc_tf_fe/utils.py b/tools/ovc/unit_tests/moc_tf_fe/utils.py index 90ce7a3e5d2c8b..75f0c83f6f0614 100644 --- a/tools/ovc/unit_tests/moc_tf_fe/utils.py +++ b/tools/ovc/unit_tests/moc_tf_fe/utils.py @@ -17,7 +17,7 @@ def basic_check(input_model, argv_input, input_data, expected_dtype, expected_va else: input_model = os.path.join(path, "test_models", input_model) - ov_model = convert_model(input_model, input=argv_input, extensions=extensions) + ov_model = convert_model(input_model, input=argv_input, extension=extensions) if only_conversion: return ov_model diff --git a/tools/ovc/unit_tests/ovc/convert/meta_data_test.py b/tools/ovc/unit_tests/ovc/convert/meta_data_test.py index e000edcaf8f8a6..60261fc8c88d4a 100644 --- a/tools/ovc/unit_tests/ovc/convert/meta_data_test.py +++ b/tools/ovc/unit_tests/ovc/convert/meta_data_test.py @@ -71,7 +71,7 @@ def check_meta_data(ov_model): if key == 'conversion_parameters': for param_name, param_value in value.items(): val = ov_model.get_rt_info([key, param_name]).astype(str) - if param_name in ['extensions', 'caffe_parser_path', 'input_model', 'k', 'output_dir']: + if param_name in ['extension', 'caffe_parser_path', 'input_model', 'k', 'output_dir']: val = Path(val) assert val == param_value, \ "Runtime info attribute with name {} does not match. Expected: {}, " \ diff --git a/tools/ovc/unit_tests/ovc/convert/meta_data_test_actual.py b/tools/ovc/unit_tests/ovc/convert/meta_data_test_actual.py index 1ba198e14c8df1..006aa997bc7c12 100644 --- a/tools/ovc/unit_tests/ovc/convert/meta_data_test_actual.py +++ b/tools/ovc/unit_tests/ovc/convert/meta_data_test_actual.py @@ -21,7 +21,7 @@ def check_meta_data(ov_model, ref_meta): if key == 'conversion_parameters': for param_name, param_value in value.items(): val = ov_model.get_rt_info([key, param_name]).astype(str) - if param_name in ['extensions', 'caffe_parser_path', 'input_model', 'k', 'output_dir']: + if param_name in ['extension', 'caffe_parser_path', 'input_model', 'k', 'output_dir']: val = Path(val) assert val == param_value, \ "Runtime info attribute with name {} does not match. Expected: {}, " \ diff --git a/tools/ovc/unit_tests/ovc/utils/args_to_string_test.py b/tools/ovc/unit_tests/ovc/utils/args_to_string_test.py deleted file mode 100644 index 4b265647c7b0cc..00000000000000 --- a/tools/ovc/unit_tests/ovc/utils/args_to_string_test.py +++ /dev/null @@ -1,20 +0,0 @@ -# Copyright (C) 2018-2023 Intel Corporation -# SPDX-License-Identifier: Apache-2.0 - -import numpy as np -from openvino.runtime import Layout, Dimension - -from openvino.tools.ovc.cli_parser import str_list_to_str -from unit_tests.ovc.unit_test_with_mocked_telemetry import UnitTestWithMockedTelemetry - - -class TestConvertingConvertArgumentsToString(UnitTestWithMockedTelemetry): - def test_str_list_to_str(self): - list_str = ["data1", "data2", "data3"] - self.assertTrue(str_list_to_str(list_str) == "data1,data2,data3") - - list_str = "data1" - self.assertTrue(str_list_to_str(list_str) == "data1") - - self.assertRaises(Exception, str_list_to_str, **{"values": [int, 1]}) - self.assertRaises(Exception, str_list_to_str, **{"values": Dimension(1)}) diff --git a/tools/ovc/unit_tests/ovc/utils/cli_parser_test.py b/tools/ovc/unit_tests/ovc/utils/cli_parser_test.py index a805649c67f1e1..629c759f33fbbb 100644 --- a/tools/ovc/unit_tests/ovc/utils/cli_parser_test.py +++ b/tools/ovc/unit_tests/ovc/utils/cli_parser_test.py @@ -13,8 +13,8 @@ import numpy as np from openvino.tools.ovc.cli_parser import input_to_input_cut_info, \ - check_positive, writable_dir, readable_dirs, \ - readable_file, get_freeze_placeholder_values, get_all_cli_parser, \ + check_positive, writable_dir, \ + readable_file_or_object, get_freeze_placeholder_values, get_all_cli_parser, \ get_mo_convert_params from openvino.tools.ovc.convert_impl import pack_params_to_args_namespace from openvino.tools.ovc.error import Error @@ -471,38 +471,30 @@ def test_multiple_writable_dirs(self): def test_single_writable_non_existing_dir(self): self.assertEqual(__class__.WRITABLE_NON_EXISTING_DIR, writable_dir(__class__.WRITABLE_NON_EXISTING_DIR)) - def test_readable_dirs(self): - dirs_str = ','.join([__class__.WRITABLE_DIR, __class__.READABLE_DIR]) - self.assertEqual(dirs_str, readable_dirs(dirs_str)) - - def test_not_readable_dirs(self): - dirs_str = ','.join([__class__.WRITABLE_DIR, __class__.WRITABLE_NON_EXISTING_DIR]) - with self.assertRaises(Error) as cm: - readable_dirs(dirs_str) def test_readable_file(self): - self.assertEqual(__class__.EXISTING_FILE, readable_file(__class__.EXISTING_FILE)) + self.assertEqual(__class__.EXISTING_FILE, readable_file_or_object(__class__.EXISTING_FILE)) def test_non_readable_file(self): with self.assertRaises(Error) as cm: - readable_file(__class__.NOT_EXISTING_FILE) + readable_file_or_object(__class__.NOT_EXISTING_FILE) class TestPackParamsToArgsNamespace(unittest.TestCase): def test_mo_convert_params(self): from openvino.frontend import ConversionExtension args = {'input_model': os.path.dirname(__file__), - 'extensions': ConversionExtension("Ext", lambda x: x), + 'extension': ConversionExtension("Ext", lambda x: x), 'input': ['name', InputCutInfo("a", [1,2,3], numpy.float32, [5, 6, 7])], 'output': ["a", "b", "c"]} cli_parser = get_all_cli_parser() - argv = pack_params_to_args_namespace(args, cli_parser) + argv = pack_params_to_args_namespace(args, cli_parser, True) assert argv.input_model == args['input_model'] - assert argv.extensions == [args['extensions']] + assert argv.extension == args['extension'] assert argv.input == ['name', InputCutInfo("a", [1,2,3], numpy.float32, [5, 6, 7])] - assert argv.output == "a,b,c" + assert argv.output == ["a","b","c"] for arg, value in vars(argv).items(): if arg not in args and arg != 'is_python_api_used': @@ -512,8 +504,9 @@ def test_not_existing_dir(self): args = {"input_model": "abc"} cli_parser = get_all_cli_parser() - with self.assertRaisesRegex(Error, "The \"abc\" is not existing file or directory"): - pack_params_to_args_namespace(args, cli_parser) + with self.assertRaisesRegex(Error, "The value for parameter \"input_model\" must be existing file/directory, " + "but \"abc\" does not exist."): + pack_params_to_args_namespace(args, cli_parser, True) def test_unknown_params(self): args = {"input_model": os.path.dirname(__file__), @@ -521,15 +514,14 @@ def test_unknown_params(self): cli_parser = get_all_cli_parser() with self.assertRaisesRegex(Error, "Unrecognized argument: a"): - pack_params_to_args_namespace(args, cli_parser) + pack_params_to_args_namespace(args, cli_parser, True) class TestConvertModelParamsParsing(unittest.TestCase): def test_mo_convert_params_parsing(self): ref_params = { - 'Framework-agnostic parameters:': {'input_model', 'input', 'output', 'example_input', - 'extensions', 'verbose', 'share_weights'}, - 'TensorFlow*-specific parameters:': {'saved_model_tags'}, + 'Optional parameters:': {'input_model', 'input', 'output', 'example_input', + 'extension', 'verbose', 'share_weights'}, 'PaddlePaddle-specific parameters:': {'example_output'}, }