From c94621ea506523b75eb45fa4f63eb0a9bd0c87de Mon Sep 17 00:00:00 2001 From: braf Date: Mon, 16 Oct 2023 16:22:06 +0000 Subject: [PATCH 1/7] Created a new class ConfigRangeNumeric and using it for periodic-concurrency --- .../perf_analyzer_config_generator.py | 5 +- .../config/input/config_command_profile.py | 9 +- .../config/input/config_list_numeric.py | 9 +- .../config/input/config_range_numeric.py | 155 ++++++++++++++++++ tests/test_cli.py | 10 +- 5 files changed, 179 insertions(+), 9 deletions(-) create mode 100644 model_analyzer/config/input/config_range_numeric.py diff --git a/model_analyzer/config/generate/perf_analyzer_config_generator.py b/model_analyzer/config/generate/perf_analyzer_config_generator.py index 771e895f1..9987e3529 100755 --- a/model_analyzer/config/generate/perf_analyzer_config_generator.py +++ b/model_analyzer/config/generate/perf_analyzer_config_generator.py @@ -215,8 +215,9 @@ def _create_input_dict(self, model_perf_analyzer_flags: Dict) -> Dict: return {} def _create_inference_load_list(self) -> List[int]: - # The two possible inference loads are request rate or concurrency - # Concurrency is the default and will be used unless the user specifies + # The three possible inference loads are request rate, concurrency or periodic concurrency + # For LLM models periodic concurrency is used for non-LLM models + # concurrency is the default and will be used unless the user specifies # request rate, either as a model parameter or a config option if self._cli_config.is_llm_model(): return self._create_periodic_concurrency_list() diff --git a/model_analyzer/config/input/config_command_profile.py b/model_analyzer/config/input/config_command_profile.py index bdce45027..d60ce4279 100755 --- a/model_analyzer/config/input/config_command_profile.py +++ b/model_analyzer/config/input/config_command_profile.py @@ -46,6 +46,7 @@ from .config_none import ConfigNone from .config_object import ConfigObject from .config_primitive import ConfigPrimitive +from .config_range_numeric import ConfigRangeNumeric from .config_sweep import ConfigSweep from .config_union import ConfigUnion from .objects.config_model_profile_spec import ConfigModelProfileSpec @@ -498,7 +499,7 @@ def _add_profile_models_configs(self): schema={ "batch_sizes": ConfigListNumeric(type_=int), "concurrency": ConfigListNumeric(type_=int), - "periodic_concurrency": ConfigListNumeric(type_=int), + "periodic_concurrency": ConfigRangeNumeric(type_=int), "request_rate": ConfigListNumeric(type_=int), "request_period": ConfigListNumeric(type_=int), "text_input_length": ConfigListNumeric(type_=int), @@ -569,9 +570,9 @@ def _add_profile_models_configs(self): ConfigField( "periodic_concurrency", flags=["--periodic-concurrency"], - field_type=ConfigListNumeric(int), - description="Comma-delimited list of periodic concurrency values or ranges " - " to be used during profiling", + field_type=ConfigRangeNumeric(int), + default_value="1:1:1", + description="Ranges to be used during profiling", ) ) self._add_config( diff --git a/model_analyzer/config/input/config_list_numeric.py b/model_analyzer/config/input/config_list_numeric.py index 799cbdf9e..b677bcdab 100755 --- a/model_analyzer/config/input/config_list_numeric.py +++ b/model_analyzer/config/input/config_list_numeric.py @@ -103,7 +103,14 @@ def set_value(self, value): try: if self._is_string(value): self._value = [] - value = value.split(",") + if "," in value: + value = value.split(",") + elif ":" in value: + value = value.split(":") + if len(value) == 2: + value = {"start": value[0], "stop": value[1], "step": 1} + else: + value = {"start": value[0], "stop": value[1], "step": value[2]} if self._is_list(value): new_value = self._process_list(value) diff --git a/model_analyzer/config/input/config_range_numeric.py b/model_analyzer/config/input/config_range_numeric.py new file mode 100644 index 000000000..54fee7b1b --- /dev/null +++ b/model_analyzer/config/input/config_range_numeric.py @@ -0,0 +1,155 @@ +# Copyright 2021-2023, NVIDIA CORPORATION & AFFILIATES. 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. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from model_analyzer.constants import CONFIG_PARSER_FAILURE, CONFIG_PARSER_SUCCESS + +from .config_status import ConfigStatus +from .config_value import ConfigValue + + +class ConfigRangeNumeric(ConfigValue): + """ + A range of numeric values. + """ + + def __init__( + self, + type_, + preprocess=None, + required=False, + validator=None, + output_mapper=None, + name=None, + ): + """ + Create a new range of numeric values. + + Parameters + ---------- + type_ : type + The type of elements in the list + preprocess : callable + Function be called before setting new values. + required : bool + Whether a given config is required or not. + validator : callable or None + A validator for the final value of the field. + output_mapper: callable or None + This callable unifies the output value of this field. + name : str + Fully qualified name for this field. + """ + + # default validator + if validator is None: + + def validator(x): + if type(x) is list: + return ConfigStatus(CONFIG_PARSER_SUCCESS) + + return ConfigStatus( + CONFIG_PARSER_FAILURE, + f'The value for field "{self.name()}" should be a list' + " and the length must be larger than zero.", + ) + + super().__init__(preprocess, required, validator, output_mapper, name) + self._type = type_ + self._cli_type = str + self._value = [] + + def set_value(self, value): + """ + Set the value for this field. + + Parameters + ---------- + value : object + The value for this field. It can be comma delimited list, or an + array, or a range + """ + + type_ = self._type + new_value = [] + + try: + if self._is_string(value): + if not ":" in value: + return ConfigStatus( + CONFIG_PARSER_FAILURE, + f'When a string is used for field "{self.name()}",' + ' it must be in the format "start:stop:step".', + config_object=self, + ) + + self._value = [] + value = value.split(":") + if len(value) == 2: + value = {"start": value[0], "stop": value[1], "step": 1} + elif len(value) == 3: + value = {"start": value[0], "stop": value[1], "step": value[2]} + else: + return ConfigStatus( + CONFIG_PARSER_FAILURE, + f'When a string is used for field "{self.name()}",' + ' it must be in the format "start:stop:step".', + config_object=self, + ) + + if self._is_dict(value): + two_key_condition = ( + len(value) == 2 and "start" in value and "stop" in value + ) + three_key_condition = ( + len(value) == 3 + and "start" in value + and "stop" in value + and "step" in value + ) + if two_key_condition or three_key_condition: + step = 1 + start = int(value["start"]) + stop = int(value["stop"]) + if start > stop: + return ConfigStatus( + CONFIG_PARSER_FAILURE, + f'When a dictionary is used for field "{self.name()}",' + ' "start" should be less than "stop".' + f" Current value is {value}.", + config_object=self, + ) + + if "step" in value: + step = int(value["step"]) + new_value = list(range(start, stop + 1, step)) + else: + return ConfigStatus( + CONFIG_PARSER_FAILURE, + f'If a dictionary is used for field "{self.name()}", it' + ' should only contain "start" and "stop" key with an' + f' optional "step" key. Currently, contains {list(value)}.', + config_object=self, + ) + elif self._is_list(value): + if not value: + assert "This should only be reached by the default case - which is an empty list" + + new_value = [] + else: + new_value = [type_(value)] + except ValueError as e: + message = f'Failed to set the value for field "{self.name()}". Error: {e}.' + return ConfigStatus(CONFIG_PARSER_FAILURE, message, self) + + return super().set_value(new_value) diff --git a/tests/test_cli.py b/tests/test_cli.py index c6669b2c2..9d71fdf06 100755 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -145,7 +145,7 @@ def get_test_options(): # expected_default_value OptionStruct("intlist", "profile", "--batch-sizes", "-b", "2, 4, 6", "1"), OptionStruct("intlist", "profile", "--concurrency", "-c", "1, 2, 3", None), - OptionStruct("intlist", "profile", "--periodic-concurrency", None, "1, 2, 3", None), + OptionStruct("intlist", "profile", "--periodic-concurrency", None, "10:100:5", None), OptionStruct("intlist", "profile", "--request-rate", None, "1, 2, 3", None), OptionStruct("intlist", "profile", "--request-period", None, "1, 2, 3", None), OptionStruct("intlist", "profile", "--text-input-length", None, "1, 2, 3", None), @@ -603,9 +603,15 @@ def _convert_string_to_numeric(self, number): return float(number) if "." in number else int(number) def _convert_string_to_int_list(self, list_values): - ret_val = [int(x) for x in list_values.split(",")] + if ":" in list_values: + ret_val = [int(x) for x in list_values.split(":")] + ret_val = list(range(ret_val[0], ret_val[1] + 1, ret_val[2])) + else: + ret_val = [int(x) for x in list_values.split(",")] + if len(ret_val) == 1: return ret_val[0] + return ret_val def _convert_string_to_string_list(self, list_values): From 22b922c68e90586520fde67598d30e91080cae9d Mon Sep 17 00:00:00 2001 From: braf Date: Mon, 16 Oct 2023 18:05:53 +0000 Subject: [PATCH 2/7] Fixes and defaults for periodic concurrency --- model_analyzer/config/input/config_command_profile.py | 2 +- model_analyzer/config/input/config_defaults.py | 1 + model_analyzer/config/input/config_range_numeric.py | 5 +---- tests/common/test_utils.py | 6 +++--- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/model_analyzer/config/input/config_command_profile.py b/model_analyzer/config/input/config_command_profile.py index d60ce4279..d2731ca2f 100755 --- a/model_analyzer/config/input/config_command_profile.py +++ b/model_analyzer/config/input/config_command_profile.py @@ -571,7 +571,7 @@ def _add_profile_models_configs(self): "periodic_concurrency", flags=["--periodic-concurrency"], field_type=ConfigRangeNumeric(int), - default_value="1:1:1", + default_value=config_defaults.DEFAULT_RUN_CONFIG_PERIODIC_CONCURRENCY, description="Ranges to be used during profiling", ) ) diff --git a/model_analyzer/config/input/config_defaults.py b/model_analyzer/config/input/config_defaults.py index bab62a4fd..aad674838 100755 --- a/model_analyzer/config/input/config_defaults.py +++ b/model_analyzer/config/input/config_defaults.py @@ -45,6 +45,7 @@ DEFAULT_CLIENT_PROTOCOL = "grpc" DEFAULT_RUN_CONFIG_MAX_CONCURRENCY = 1024 DEFAULT_RUN_CONFIG_MIN_CONCURRENCY = 1 +DEFAULT_RUN_CONFIG_PERIODIC_CONCURRENCY = "1:1:1" DEFAULT_RUN_CONFIG_MAX_PERIODIC_CONCURRENCY = 1024 DEFAULT_RUN_CONFIG_MIN_PERIODIC_CONCURRENCY = 16 DEFAULT_RUN_CONFIG_MAX_PERIODIC_CONCURRENCY_STEP = 128 diff --git a/model_analyzer/config/input/config_range_numeric.py b/model_analyzer/config/input/config_range_numeric.py index 54fee7b1b..cd7566a52 100644 --- a/model_analyzer/config/input/config_range_numeric.py +++ b/model_analyzer/config/input/config_range_numeric.py @@ -142,10 +142,7 @@ def set_value(self, value): config_object=self, ) elif self._is_list(value): - if not value: - assert "This should only be reached by the default case - which is an empty list" - - new_value = [] + new_value = value else: new_value = [type_(value)] except ValueError as e: diff --git a/tests/common/test_utils.py b/tests/common/test_utils.py index 380a5d404..e8448ae98 100755 --- a/tests/common/test_utils.py +++ b/tests/common/test_utils.py @@ -29,7 +29,7 @@ DEFAULT_OUTPUT_MODEL_REPOSITORY, DEFAULT_RUN_CONFIG_MIN_CONCURRENCY, DEFAULT_RUN_CONFIG_MIN_MAX_TOKEN_COUNT, - DEFAULT_RUN_CONFIG_MIN_PERIODIC_CONCURRENCY, + DEFAULT_RUN_CONFIG_PERIODIC_CONCURRENCY, DEFAULT_TRITON_GRPC_ENDPOINT, DEFAULT_TRITON_HTTP_ENDPOINT, DEFAULT_TRITON_INSTALL_PATH, @@ -241,7 +241,7 @@ def construct_perf_analyzer_config( export_file_name="my-model-results.json", batch_size=DEFAULT_BATCH_SIZES, concurrency=DEFAULT_RUN_CONFIG_MIN_CONCURRENCY, - periodic_concurrency=DEFAULT_RUN_CONFIG_MIN_PERIODIC_CONCURRENCY, + periodic_concurrency=DEFAULT_RUN_CONFIG_PERIODIC_CONCURRENCY, request_rate=None, max_token_count=DEFAULT_RUN_CONFIG_MIN_MAX_TOKEN_COUNT, launch_mode=DEFAULT_TRITON_LAUNCH_MODE, @@ -264,7 +264,7 @@ def construct_perf_analyzer_config( The batch size for this PA configuration concurrency: int The concurrency value for this PA configuration - periodic_concurrency: + periodic_concurrency: list The periodic concurrency value for this PA configuration request_rate: int The request rate value for this PA configuration From 21919ac1322d4fbd98094a89a0e7fc69629260ca Mon Sep 17 00:00:00 2001 From: braf Date: Mon, 16 Oct 2023 18:33:03 +0000 Subject: [PATCH 3/7] First unit test passing --- .../config/input/config_range_numeric.py | 7 ++-- tests/test_perf_analyzer_config_generator.py | 37 +++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/model_analyzer/config/input/config_range_numeric.py b/model_analyzer/config/input/config_range_numeric.py index cd7566a52..51a1aa2f9 100644 --- a/model_analyzer/config/input/config_range_numeric.py +++ b/model_analyzer/config/input/config_range_numeric.py @@ -118,9 +118,10 @@ def set_value(self, value): and "step" in value ) if two_key_condition or three_key_condition: - step = 1 start = int(value["start"]) stop = int(value["stop"]) + step = 1 if not "step" in value else int(value["step"]) + if start > stop: return ConfigStatus( CONFIG_PARSER_FAILURE, @@ -130,9 +131,7 @@ def set_value(self, value): config_object=self, ) - if "step" in value: - step = int(value["step"]) - new_value = list(range(start, stop + 1, step)) + new_value = [f"{start}:{stop}:{step}"] else: return ConfigStatus( CONFIG_PARSER_FAILURE, diff --git a/tests/test_perf_analyzer_config_generator.py b/tests/test_perf_analyzer_config_generator.py index f00084335..9ce744c86 100755 --- a/tests/test_perf_analyzer_config_generator.py +++ b/tests/test_perf_analyzer_config_generator.py @@ -627,6 +627,43 @@ def test_llm_search_text_input_length(self): yaml_str, expected_configs, pa_cli_args ) + def test_periodic_concurrency_parameter(self): + """ + Test LLM Search: + - periodic-concurrency: 10:100:10 + + Max token set to 1 + Text input set to 1 + """ + + # yapf: disable + yaml_str = (""" + perf_analyzer_flags: + input-data: input-data.json + profile_models: + - my-model + """) + # yapf: enable + + expected_configs = [ + construct_perf_analyzer_config( + llm_search_mode=True, periodic_concurrency="10:100:10" + ) + ] + + pa_cli_args = [ + "--llm-search-enable", + "--periodic-concurrency", + "10:100:10", + "--run-config-search-max-max-token-count", + "1", + "--run-config-search-max-text-input-length", + "1", + ] + self._run_and_test_perf_analyzer_config_generator( + yaml_str, expected_configs, pa_cli_args + ) + def test_perf_analyzer_config_ssl_options(self): """ Test Perf Analyzer SSL options: From d9e076f5374d3be806fa34b42a6094b7ab5e3b59 Mon Sep 17 00:00:00 2001 From: braf Date: Tue, 17 Oct 2023 17:42:32 +0000 Subject: [PATCH 4/7] PACG chagnes complete. Unit tests updated and passing --- .../perf_analyzer_config_generator.py | 56 ++++++++++--- .../config/input/config_command_profile.py | 8 +- tests/test_cli.py | 2 +- tests/test_perf_analyzer_config_generator.py | 83 ++++++++++++++++--- 4 files changed, 123 insertions(+), 26 deletions(-) diff --git a/model_analyzer/config/generate/perf_analyzer_config_generator.py b/model_analyzer/config/generate/perf_analyzer_config_generator.py index 9987e3529..be352ba6b 100755 --- a/model_analyzer/config/generate/perf_analyzer_config_generator.py +++ b/model_analyzer/config/generate/perf_analyzer_config_generator.py @@ -16,8 +16,8 @@ import json import logging -from itertools import repeat -from typing import Dict, Generator, List, Optional, Tuple +from itertools import permutations, repeat +from typing import Any, Dict, Generator, List, Optional, Tuple from model_analyzer.config.input.config_command_profile import ConfigCommandProfile from model_analyzer.config.input.config_defaults import ( @@ -214,7 +214,7 @@ def _create_input_dict(self, model_perf_analyzer_flags: Dict) -> Dict: else: return {} - def _create_inference_load_list(self) -> List[int]: + def _create_inference_load_list(self) -> List[Any]: # The three possible inference loads are request rate, concurrency or periodic concurrency # For LLM models periodic concurrency is used for non-LLM models # concurrency is the default and will be used unless the user specifies @@ -248,16 +248,52 @@ def _create_concurrency_list(self) -> List[int]: self._cli_config.run_config_search_max_concurrency, ) - def _create_periodic_concurrency_list(self) -> List[int]: + def _create_periodic_concurrency_list(self) -> List[str]: if self._model_parameters["periodic_concurrency"]: return sorted(self._model_parameters["periodic_concurrency"]) elif self._cli_config.run_config_search_disable: - return [DEFAULT_RUN_CONFIG_MIN_PERIODIC_CONCURRENCY] - else: - return utils.generate_doubled_list( - self._cli_config.run_config_search_min_periodic_concurrency, - self._cli_config.run_config_search_max_periodic_concurrency, - ) + return [DEFAULT_RUN_CONFIG_PERIODIC_CONCURRENCY] + + periodic_concurrencies = self._generate_periodic_concurrencies() + return periodic_concurrencies + + def _generate_periodic_concurrencies(self) -> List[str]: + periodic_concurrencies = [] + + periodic_concurrency_doubled_list = utils.generate_doubled_list( + self._cli_config.run_config_search_min_periodic_concurrency, + self._cli_config.run_config_search_max_periodic_concurrency, + ) + + step_doubled_list = utils.generate_doubled_list( + self._cli_config.run_config_search_min_periodic_concurrency_step, + self._cli_config.run_config_search_max_periodic_concurrency_step, + ) + + for min_periodic_concurrency in periodic_concurrency_doubled_list: + for max_periodic_concurrency in periodic_concurrency_doubled_list: + for step in step_doubled_list: + if self._is_illegal_periodic_concurrency_combination( + min_periodic_concurrency, max_periodic_concurrency, step + ): + continue + + periodic_concurrencies.append( + f"{min_periodic_concurrency}:{max_periodic_concurrency}:{step}" + ) + return periodic_concurrencies + + def _is_illegal_periodic_concurrency_combination( + self, min: int, max: int, step: int + ) -> bool: + if min > max: + return True + elif max == min and step != 1: + return True + elif (max - min) % step: + return True + + return False def _create_text_input_length_list(self) -> List[int]: if not self._cli_config.is_llm_model(): diff --git a/model_analyzer/config/input/config_command_profile.py b/model_analyzer/config/input/config_command_profile.py index d2731ca2f..9da3e7d31 100755 --- a/model_analyzer/config/input/config_command_profile.py +++ b/model_analyzer/config/input/config_command_profile.py @@ -46,7 +46,6 @@ from .config_none import ConfigNone from .config_object import ConfigObject from .config_primitive import ConfigPrimitive -from .config_range_numeric import ConfigRangeNumeric from .config_sweep import ConfigSweep from .config_union import ConfigUnion from .objects.config_model_profile_spec import ConfigModelProfileSpec @@ -499,7 +498,7 @@ def _add_profile_models_configs(self): schema={ "batch_sizes": ConfigListNumeric(type_=int), "concurrency": ConfigListNumeric(type_=int), - "periodic_concurrency": ConfigRangeNumeric(type_=int), + "periodic_concurrency": ConfigListString(), "request_rate": ConfigListNumeric(type_=int), "request_period": ConfigListNumeric(type_=int), "text_input_length": ConfigListNumeric(type_=int), @@ -570,9 +569,8 @@ def _add_profile_models_configs(self): ConfigField( "periodic_concurrency", flags=["--periodic-concurrency"], - field_type=ConfigRangeNumeric(int), - default_value=config_defaults.DEFAULT_RUN_CONFIG_PERIODIC_CONCURRENCY, - description="Ranges to be used during profiling", + field_type=ConfigListString(), + description="A list of ranges to be used during profiling", ) ) self._add_config( diff --git a/tests/test_cli.py b/tests/test_cli.py index 9d71fdf06..1a2fb84a2 100755 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -145,7 +145,7 @@ def get_test_options(): # expected_default_value OptionStruct("intlist", "profile", "--batch-sizes", "-b", "2, 4, 6", "1"), OptionStruct("intlist", "profile", "--concurrency", "-c", "1, 2, 3", None), - OptionStruct("intlist", "profile", "--periodic-concurrency", None, "10:100:5", None), + OptionStruct("stringlist", "profile", "--periodic-concurrency", None, '"5:50:5", "10:100:10"', None, None), OptionStruct("intlist", "profile", "--request-rate", None, "1, 2, 3", None), OptionStruct("intlist", "profile", "--request-period", None, "1, 2, 3", None), OptionStruct("intlist", "profile", "--text-input-length", None, "1, 2, 3", None), diff --git a/tests/test_perf_analyzer_config_generator.py b/tests/test_perf_analyzer_config_generator.py index 9ce744c86..ebce26095 100755 --- a/tests/test_perf_analyzer_config_generator.py +++ b/tests/test_perf_analyzer_config_generator.py @@ -577,15 +577,23 @@ def test_llm_search_max_token_count(self): # yapf: enable max_token_counts = utils.generate_doubled_list(1, 256) - expected_configs = [ - construct_perf_analyzer_config(max_token_count=mtc, llm_search_mode=True) - for mtc in max_token_counts - ] + periodic_concurrencies = ["16:32:4", "16:32:8", "16:32:16"] + + expected_configs = [] + for mtc in max_token_counts: + for pc in periodic_concurrencies: + expected_configs.append( + construct_perf_analyzer_config( + max_token_count=mtc, + llm_search_mode=True, + periodic_concurrency=pc, + ) + ) pa_cli_args = [ "--llm-search-enable", "--run-config-search-max-periodic-concurrency", - "16", + "32", "--run-config-search-max-text-input-length", "1", ] @@ -611,15 +619,21 @@ def test_llm_search_text_input_length(self): # yapf: enable text_input_lengths = utils.generate_doubled_list(1, 1024) - expected_configs = [ - construct_perf_analyzer_config(llm_search_mode=True) - for pl in text_input_lengths - ] + periodic_concurrencies = ["16:32:4", "16:32:8", "16:32:16"] + + expected_configs = [] + for til in text_input_lengths: + for pc in periodic_concurrencies: + expected_configs.append( + construct_perf_analyzer_config( + llm_search_mode=True, periodic_concurrency=pc + ) + ) pa_cli_args = [ "--llm-search-enable", "--run-config-search-max-periodic-concurrency", - "16", + "32", "--run-config-search-max-max-token-count", "1", ] @@ -664,6 +678,55 @@ def test_periodic_concurrency_parameter(self): yaml_str, expected_configs, pa_cli_args ) + def test_periodic_concurrency_search(self): + """ + Test LLM Search: + - Period Concurrency using RCS values + + Max token set to 1 + Text input set to 1 + """ + + # yapf: disable + yaml_str = (""" + perf_analyzer_flags: + input-data: input-data.json + profile_models: + - my-model + """) + # yapf: enable + + periodic_concurrencies = [ + "16:32:8", + "16:32:16", + "16:64:8", + "16:64:16", + "32:64:8", + "32:64:16", + "32:64:32", + ] + expected_configs = [ + construct_perf_analyzer_config( + llm_search_mode=True, periodic_concurrency=pc + ) + for pc in periodic_concurrencies + ] + + pa_cli_args = [ + "--llm-search-enable", + "--run-config-search-max-max-token-count", + "1", + "--run-config-search-max-text-input-length", + "1", + "--run-config-search-max-periodic-concurrency", + "64", + "--run-config-search-min-periodic-concurrency-step", + "8", + ] + self._run_and_test_perf_analyzer_config_generator( + yaml_str, expected_configs, pa_cli_args + ) + def test_perf_analyzer_config_ssl_options(self): """ Test Perf Analyzer SSL options: From daccc307aa16b0270d93a23af1db4c9e1b5bad96 Mon Sep 17 00:00:00 2001 From: braf Date: Tue, 17 Oct 2023 18:18:24 +0000 Subject: [PATCH 5/7] Removing uneeded class --- .../config/input/config_range_numeric.py | 151 ------------------ 1 file changed, 151 deletions(-) delete mode 100644 model_analyzer/config/input/config_range_numeric.py diff --git a/model_analyzer/config/input/config_range_numeric.py b/model_analyzer/config/input/config_range_numeric.py deleted file mode 100644 index 51a1aa2f9..000000000 --- a/model_analyzer/config/input/config_range_numeric.py +++ /dev/null @@ -1,151 +0,0 @@ -# Copyright 2021-2023, NVIDIA CORPORATION & AFFILIATES. 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. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from model_analyzer.constants import CONFIG_PARSER_FAILURE, CONFIG_PARSER_SUCCESS - -from .config_status import ConfigStatus -from .config_value import ConfigValue - - -class ConfigRangeNumeric(ConfigValue): - """ - A range of numeric values. - """ - - def __init__( - self, - type_, - preprocess=None, - required=False, - validator=None, - output_mapper=None, - name=None, - ): - """ - Create a new range of numeric values. - - Parameters - ---------- - type_ : type - The type of elements in the list - preprocess : callable - Function be called before setting new values. - required : bool - Whether a given config is required or not. - validator : callable or None - A validator for the final value of the field. - output_mapper: callable or None - This callable unifies the output value of this field. - name : str - Fully qualified name for this field. - """ - - # default validator - if validator is None: - - def validator(x): - if type(x) is list: - return ConfigStatus(CONFIG_PARSER_SUCCESS) - - return ConfigStatus( - CONFIG_PARSER_FAILURE, - f'The value for field "{self.name()}" should be a list' - " and the length must be larger than zero.", - ) - - super().__init__(preprocess, required, validator, output_mapper, name) - self._type = type_ - self._cli_type = str - self._value = [] - - def set_value(self, value): - """ - Set the value for this field. - - Parameters - ---------- - value : object - The value for this field. It can be comma delimited list, or an - array, or a range - """ - - type_ = self._type - new_value = [] - - try: - if self._is_string(value): - if not ":" in value: - return ConfigStatus( - CONFIG_PARSER_FAILURE, - f'When a string is used for field "{self.name()}",' - ' it must be in the format "start:stop:step".', - config_object=self, - ) - - self._value = [] - value = value.split(":") - if len(value) == 2: - value = {"start": value[0], "stop": value[1], "step": 1} - elif len(value) == 3: - value = {"start": value[0], "stop": value[1], "step": value[2]} - else: - return ConfigStatus( - CONFIG_PARSER_FAILURE, - f'When a string is used for field "{self.name()}",' - ' it must be in the format "start:stop:step".', - config_object=self, - ) - - if self._is_dict(value): - two_key_condition = ( - len(value) == 2 and "start" in value and "stop" in value - ) - three_key_condition = ( - len(value) == 3 - and "start" in value - and "stop" in value - and "step" in value - ) - if two_key_condition or three_key_condition: - start = int(value["start"]) - stop = int(value["stop"]) - step = 1 if not "step" in value else int(value["step"]) - - if start > stop: - return ConfigStatus( - CONFIG_PARSER_FAILURE, - f'When a dictionary is used for field "{self.name()}",' - ' "start" should be less than "stop".' - f" Current value is {value}.", - config_object=self, - ) - - new_value = [f"{start}:{stop}:{step}"] - else: - return ConfigStatus( - CONFIG_PARSER_FAILURE, - f'If a dictionary is used for field "{self.name()}", it' - ' should only contain "start" and "stop" key with an' - f' optional "step" key. Currently, contains {list(value)}.', - config_object=self, - ) - elif self._is_list(value): - new_value = value - else: - new_value = [type_(value)] - except ValueError as e: - message = f'Failed to set the value for field "{self.name()}". Error: {e}.' - return ConfigStatus(CONFIG_PARSER_FAILURE, message, self) - - return super().set_value(new_value) From d7fb6623bd4b78727ee9ddd660e9bfbb32a31af5 Mon Sep 17 00:00:00 2001 From: braf Date: Tue, 17 Oct 2023 22:20:56 +0000 Subject: [PATCH 6/7] Fixing codeQL and hwoo's review suggestions --- .../perf_analyzer_config_generator.py | 22 +++++++++---------- tests/test_perf_analyzer_config_generator.py | 2 +- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/model_analyzer/config/generate/perf_analyzer_config_generator.py b/model_analyzer/config/generate/perf_analyzer_config_generator.py index be352ba6b..f60553eef 100755 --- a/model_analyzer/config/generate/perf_analyzer_config_generator.py +++ b/model_analyzer/config/generate/perf_analyzer_config_generator.py @@ -16,7 +16,7 @@ import json import logging -from itertools import permutations, repeat +from itertools import repeat from typing import Any, Dict, Generator, List, Optional, Tuple from model_analyzer.config.input.config_command_profile import ConfigCommandProfile @@ -24,9 +24,9 @@ DEFAULT_INPUT_JSON_PATH, DEFAULT_RUN_CONFIG_MIN_CONCURRENCY, DEFAULT_RUN_CONFIG_MIN_MAX_TOKEN_COUNT, - DEFAULT_RUN_CONFIG_MIN_PERIODIC_CONCURRENCY, DEFAULT_RUN_CONFIG_MIN_REQUEST_RATE, DEFAULT_RUN_CONFIG_MIN_TEXT_INPUT_LENGTH, + DEFAULT_RUN_CONFIG_PERIODIC_CONCURRENCY, ) from model_analyzer.constants import ( LOGGER_NAME, @@ -270,27 +270,25 @@ def _generate_periodic_concurrencies(self) -> List[str]: self._cli_config.run_config_search_max_periodic_concurrency_step, ) - for min_periodic_concurrency in periodic_concurrency_doubled_list: - for max_periodic_concurrency in periodic_concurrency_doubled_list: + for start in periodic_concurrency_doubled_list: + for end in periodic_concurrency_doubled_list: for step in step_doubled_list: if self._is_illegal_periodic_concurrency_combination( - min_periodic_concurrency, max_periodic_concurrency, step + start, end, step ): continue - periodic_concurrencies.append( - f"{min_periodic_concurrency}:{max_periodic_concurrency}:{step}" - ) + periodic_concurrencies.append(f"{start}:{end}:{step}") return periodic_concurrencies def _is_illegal_periodic_concurrency_combination( - self, min: int, max: int, step: int + self, start: int, end: int, step: int ) -> bool: - if min > max: + if start > end: return True - elif max == min and step != 1: + elif start == end and step != 1: return True - elif (max - min) % step: + elif (end - start) % step: return True return False diff --git a/tests/test_perf_analyzer_config_generator.py b/tests/test_perf_analyzer_config_generator.py index ebce26095..a405e2df6 100755 --- a/tests/test_perf_analyzer_config_generator.py +++ b/tests/test_perf_analyzer_config_generator.py @@ -622,7 +622,7 @@ def test_llm_search_text_input_length(self): periodic_concurrencies = ["16:32:4", "16:32:8", "16:32:16"] expected_configs = [] - for til in text_input_lengths: + for _ in text_input_lengths: for pc in periodic_concurrencies: expected_configs.append( construct_perf_analyzer_config( From 2ec0743003650476a3b21b2a884e9b28255cc6f9 Mon Sep 17 00:00:00 2001 From: braf Date: Tue, 17 Oct 2023 22:23:02 +0000 Subject: [PATCH 7/7] Adding missing else --- .../config/generate/perf_analyzer_config_generator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/model_analyzer/config/generate/perf_analyzer_config_generator.py b/model_analyzer/config/generate/perf_analyzer_config_generator.py index f60553eef..104ed79e6 100755 --- a/model_analyzer/config/generate/perf_analyzer_config_generator.py +++ b/model_analyzer/config/generate/perf_analyzer_config_generator.py @@ -290,8 +290,8 @@ def _is_illegal_periodic_concurrency_combination( return True elif (end - start) % step: return True - - return False + else: + return False def _create_text_input_length_list(self) -> List[int]: if not self._cli_config.is_llm_model():