Multi-model and Quick Search Config Checking (#540)

* Initial refactor

* Fixing type checking error

* Checking for illegal combinations. CLI unit test is failing

* Patching CLI unit test

* Updating copyright date

* Updates based on Tim's review comments

* Adding check that quick passes

* Added test to check for model specific parameters

* Checking profile_models for empty condition

* Fixing return condition in _check_no_per_model_list_values

* Adding check for model config parameters

* Making it an error to disable run config from quick search

Author: nv-braf

model_analyzer/config/input/config_command.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2021, NVIDIA CORPORATION. All rights reserved.
+# Copyright (c) 2021-22, NVIDIA CORPORATION. All rights reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -12,10 +12,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from typing import Dict, List
+from typing import Dict, List, Optional, Any
 from model_analyzer.model_analyzer_exceptions \
     import TritonModelAnalyzerException
 import yaml
+from argparse import Namespace
 from .yaml_config_validator import YamlConfigValidator
 
 from copy import deepcopy
@@ -76,7 +77,7 @@ def _load_config_file(self, file_path):
             config = yaml.safe_load(config_file)
             return config
 
-    def set_config_values(self, args):
+    def set_config_values(self, args: Namespace) -> None:
         """
         Set the config values. This function sets all the values for the
         config. CLI arguments have the highest priority, then YAML config
@@ -94,41 +95,183 @@ def set_config_values(self, args):
             this exception
         """
 
-        # Config file has been specified
+        yaml_config = self._load_yaml_config(args)
+        self._check_for_illegal_config_settings(args, yaml_config)
+        self._set_field_values(args, yaml_config)
+        self._preprocess_and_verify_arguments()
+        self._autofill_values()
+
+    def _load_yaml_config(self, args: Namespace) -> Optional[Dict[str, List]]:
         if 'config_file' in args:
             yaml_config = self._load_config_file(args.config_file)
             YamlConfigValidator.validate(yaml_config)
         else:
             yaml_config = None
 
+        return yaml_config
+
+    def _check_for_illegal_config_settings(
+            self, args: Namespace, yaml_config: Optional[Dict[str,
+                                                              List]]) -> None:
+        self._check_for_duplicate_profile_models_option(args, yaml_config)
+        self._check_for_multi_model_incompatability(args, yaml_config)
+        self._check_for_quick_search_incompatability(args, yaml_config)
+
+    def _set_field_values(self, args: Namespace,
+                          yaml_config: Optional[Dict[str, List]]) -> None:
         for key, value in self._fields.items():
             self._fields[key].set_name(key)
-            if key in args:
-                self._check_for_duplicate_profile_models_option(
-                    yaml_config, key)
-                self._fields[key].set_value(getattr(args, key))
-            elif yaml_config is not None and key in yaml_config:
-                self._fields[key].set_value(yaml_config[key])
+            config_value = self._get_config_value(key, args, yaml_config)
+
+            if config_value:
+                self._fields[key].set_value(config_value)
             elif value.default_value() is not None:
                 self._fields[key].set_value(value.default_value())
             elif value.required():
                 flags = ', '.join(value.flags())
                 raise TritonModelAnalyzerException(
                     f'Config for {value.name()} is not specified. You need to specify it using the YAML config file or using the {flags} flags in CLI.'
                 )
-        self._preprocess_and_verify_arguments()
-        self._autofill_values()
 
-    def _check_for_duplicate_profile_models_option(self,
-                                                   yaml_config: Dict[str, List],
-                                                   key: str) -> None:
-        if yaml_config is not None and key in yaml_config and key == 'profile_models':
+    def _get_config_value(
+            self, key: str, args: Namespace,
+            yaml_config: Optional[Dict[str, List]]) -> Optional[Any]:
+        if key in args:
+            return getattr(args, key)
+        elif yaml_config is not None and key in yaml_config:
+            return yaml_config[key]
+        else:
+            return None
+
+    def _check_for_duplicate_profile_models_option(
+            self, args: Namespace, yaml_config: Optional[Dict[str,
+                                                              List]]) -> None:
+        key_in_args = 'profile_models' in args
+        key_in_yaml = yaml_config is not None and 'profile_models' in yaml_config
+
+        if key_in_args and key_in_yaml:
             raise TritonModelAnalyzerException(
                 f'\n The profile model option is specified on both '
                 'the CLI (--profile-models) and in the YAML config file.'
                 '\n Please remove the option from one of the locations and try again'
             )
 
+    def _check_for_multi_model_incompatability(
+            self, args: Namespace, yaml_config: Optional[Dict[str,
+                                                              List]]) -> None:
+        if not self._get_config_value(
+                'run_config_profile_models_concurrently_enable', args,
+                yaml_config):
+            return
+
+        self._check_multi_model_search_mode_incompatability(args, yaml_config)
+
+    def _check_multi_model_search_mode_incompatability(
+            self, args: Namespace, yaml_config: Optional[Dict[str,
+                                                              List]]) -> None:
+        if self._get_config_value('run_config_search_mode', args,
+                                  yaml_config) != 'quick':
+            raise TritonModelAnalyzerException(
+                f'\nConcurrent profiling of models is only supported in quick search mode.'
+                '\nPlease use quick search mode or disable concurrent model profiling.'
+            )
+
+    def _check_for_quick_search_incompatability(
+            self, args: Namespace, yaml_config: Optional[Dict[str,
+                                                              List]]) -> None:
+        if self._get_config_value('run_config_search_mode', args,
+                                  yaml_config) != 'quick':
+            return
+
+        self._check_no_search_disable(args, yaml_config)
+        self._check_no_search_values(args, yaml_config)
+        self._check_no_global_list_values(args, yaml_config)
+        self._check_no_per_model_list_values(args, yaml_config)
+
+    def _check_no_search_disable(
+            self, args: Namespace, yaml_config: Optional[Dict[str,
+                                                              List]]) -> None:
+        if self._get_config_value('run_config_search_disable', args,
+                                  yaml_config):
+            raise TritonModelAnalyzerException(
+                f'\nDisabling of run config search is not supported in quick search mode.'
+                '\nPlease use brute search mode or remove --run-config-search-disable.'
+            )
+
+    def _check_no_search_values(self, args: Namespace,
+                                yaml_config: Optional[Dict[str, List]]) -> None:
+        max_concurrency = self._get_config_value(
+            'run_config_search_max_concurrency', args, yaml_config)
+        min_concurrency = self._get_config_value(
+            'run_config_search_min_concurrency', args, yaml_config)
+        max_instance = self._get_config_value(
+            'run_config_search_max_instance_count', args, yaml_config)
+        min_instance = self._get_config_value(
+            'run_config_search_min_instance_count', args, yaml_config)
+        max_batch_size = self._get_config_value(
+            'run_config_search_max_model_batch_size', args, yaml_config)
+        min_batch_size = self._get_config_value(
+            'run_config_search_min_model_batch_size', args, yaml_config)
+
+        if max_concurrency or min_concurrency:
+            raise TritonModelAnalyzerException(
+                f'\nProfiling of models in quick search mode is not supported with min/max concurrency search values.'
+                '\nPlease use brute search mode or remove concurrency search values.'
+            )
+        if max_instance or min_instance:
+            raise TritonModelAnalyzerException(
+                f'\nProfiling of models in quick search mode is not supported with min/max instance search values.'
+                '\nPlease use brute search mode or remove instance search values.'
+            )
+        if max_batch_size or min_batch_size:
+            raise TritonModelAnalyzerException(
+                f'\nProfiling of models in quick search mode is not supported with min/max batch size search values.'
+                '\nPlease use brute search mode or remove batch size search values.'
+            )
+
+    def _check_no_global_list_values(
+            self, args: Namespace, yaml_config: Optional[Dict[str,
+                                                              List]]) -> None:
+        concurrency = self._get_config_value('concurrency', args, yaml_config)
+        batch_sizes = self._get_config_value('batch_sizes', args, yaml_config)
+
+        if concurrency or batch_sizes:
+            raise TritonModelAnalyzerException(
+                f'\nProfiling of models in quick search mode is not supported with lists of concurrencies or batch sizes.'
+                '\nPlease use brute search mode or remove concurrency/batch sizes list.'
+            )
+
+    def _check_no_per_model_list_values(
+            self, args: Namespace, yaml_config: Optional[Dict[str,
+                                                              List]]) -> None:
+        profile_models = self._get_config_value('profile_models', args,
+                                                yaml_config)
+
+        if not profile_models or type(profile_models) is str or type(
+                profile_models) is list:
+            return
+
+        for model in profile_models.values():
+            if not 'parameters' in model:
+                continue
+
+            if 'concurrency' in model['parameters'] or 'batch size' in model[
+                    'parameters']:
+                raise TritonModelAnalyzerException(
+                    f'\nProfiling of models in quick search mode is not supported with lists of concurrencies or batch sizes.'
+                    '\nPlease use brute search mode or remove concurrency/batch sizes list.'
+                )
+
+        for model in profile_models.values():
+            if not 'model_config_parameters' in model:
+                continue
+
+            if 'max_batch_size' in model['model_config_parameters']:
+                raise TritonModelAnalyzerException(
+                    f'\nProfiling of models in quick search mode is not supported with lists max batch sizes.'
+                    '\nPlease use brute search mode or remov max batch size list.'
+                )
+
     def _preprocess_and_verify_arguments(self):
         """
         Enforces some rules on the config.

tests/test_cli.py

@@ -367,6 +367,9 @@ def test_basic_cli_config_profile_options(self):
     @patch(
         'model_analyzer.config.input.config_command_profile.ConfigCommandProfile._load_config_file',
         MagicMock())
+    @patch(
+        'model_analyzer.config.input.config_command.ConfigCommand._check_for_illegal_config_settings',
+        MagicMock())
     def test_all_options(self):
 
         options = get_test_options()

tests/test_config.py

@@ -18,6 +18,9 @@
 from .mocks.mock_numba import MockNumba
 from .mocks.mock_os import MockOSMethods
 
+from typing import Dict, List, Optional
+from argparse import Namespace
+
 from .common import test_result_collector as trc
 
 from model_analyzer.model_analyzer_exceptions \
@@ -1755,6 +1758,135 @@ def test_copy(self):
         self.assertNotEqual(configA.run_config_search_mode,
                             configB.run_config_search_mode)
 
+    def test_multi_model_search_mode(self):
+        """
+        Test that multi-model is only run in quick
+        """
+        args = [
+            'model-analzyer', 'profile', '--model-repository', 'cli-repository',
+            '--profile-models', 'test_modelA,test_modelB',
+            '--run-config-profile-models-concurrently-enable'
+        ]
+
+        yaml_content = ''
+
+        # Tests the case where no search mode is specified (default is brute)
+        with self.assertRaises(TritonModelAnalyzerException):
+            self._evaluate_config(args, yaml_content, subcommand='profile')
+
+        # Brute should fail
+        new_args = deepcopy(args)
+        new_args.append('--run-config-search-mode')
+        new_args.append('brute')
+
+        with self.assertRaises(TritonModelAnalyzerException):
+            self._evaluate_config(new_args, yaml_content, subcommand='profile')
+
+        # Quick should pass
+        new_args = deepcopy(args)
+        new_args.append('--run-config-search-mode')
+        new_args.append('quick')
+
+        self._evaluate_config(new_args, yaml_content, subcommand='profile')
+
+    def test_quick_search_mode(self):
+        """
+        Test that only legal options are specified in quick search
+        """
+        args = [
+            'model-analzyer', 'profile', '--model-repository', 'cli-repository',
+            '--profile-models', 'test_modelA', '--run-config-search-mode',
+            'quick'
+        ]
+
+        yaml_content = ''
+
+        self._evaluate_config(args, yaml_content)
+
+        self._test_quick_search_with_rcs(args,
+                                         yaml_content,
+                                         '--run-config-search-disable',
+                                         use_value=False,
+                                         use_list=False)
+        self._test_quick_search_with_rcs(args, yaml_content,
+                                         '--run-config-search-min-concurrency')
+        self._test_quick_search_with_rcs(args, yaml_content,
+                                         '--run-config-search-max-concurrency')
+        self._test_quick_search_with_rcs(
+            args, yaml_content, '--run-config-search-min-instance-count')
+        self._test_quick_search_with_rcs(
+            args, yaml_content, '--run-config-search-max-instance-count')
+        self._test_quick_search_with_rcs(
+            args, yaml_content, '--run-config-search-min-model-batch-size')
+        self._test_quick_search_with_rcs(
+            args, yaml_content, '--run-config-search-max-model-batch-size')
+        self._test_quick_search_with_rcs(args,
+                                         yaml_content,
+                                         '--batch-sizes',
+                                         use_value=False)
+        self._test_quick_search_with_rcs(args,
+                                           yaml_content,
+                                           '--concurrency',
+                                           use_value=False)
+
+    def test_quick_search_model_specific(self):
+        """
+        Test for illegal model specific options in the YAML during quick search
+        """
+        args = [
+            'model-analzyer', 'profile', '--model-repository', 'cli-repository',
+            '--run-config-search-mode', 'quick', '-f', 'path-to-config-file'
+        ]
+
+        yaml_content = """
+        profile_models:
+          model_1:
+            parameters:
+              concurrency: 1,2,4,128
+        """
+
+        with self.assertRaises(TritonModelAnalyzerException):
+            self._evaluate_config(args, yaml_content, subcommand='profile')
+
+        yaml_content = """
+        profile_models:
+          model_1:
+            parameters:
+              batch size: 1,2,4,128
+        """
+
+        with self.assertRaises(TritonModelAnalyzerException):
+            self._evaluate_config(args, yaml_content, subcommand='profile')
+
+        yaml_content = """
+        profile_models:
+          model_1:
+            model_config_parameters:
+              max batch size: 2
+        """
+
+        with self.assertRaises(TritonModelAnalyzerException):
+            self._evaluate_config(args, yaml_content, subcommand='profile')
+
+    def _test_quick_search_with_rcs(self,
+                                    args: Namespace,
+                                    yaml_content: Optional[Dict[str, List]],
+                                    rcs_string: str,
+                                    use_value: bool = True,
+                                    use_list: bool = True) -> None:
+        """
+        Tests that run-config-search options raise exceptions 
+        in quick search mode
+        """
+        new_args = deepcopy(args)
+        new_args.append(rcs_string)
+        if use_value:
+            new_args.append('1')
+        elif use_list:
+            new_args.append(['1', '2', '4'])
+        with self.assertRaises(TritonModelAnalyzerException):
+            self._evaluate_config(new_args, yaml_content, subcommand='profile')
+
 
 if __name__ == '__main__':
     unittest.main()