Adding new options for LLM #768
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
nv-braf
commented
Oct 2, 2023
| @@ -51,10 +51,15 @@ | |||
| DEFAULT_RUN_CONFIG_MIN_MODEL_BATCH_SIZE = 1 | |||
| DEFAULT_RUN_CONFIG_MAX_MODEL_BATCH_SIZE = 128 | |||
| DEFAULT_RUN_CONFIG_MAX_BINARY_SEARCH_STEPS = 5 | |||
| DEFAULT_RUN_CONFIG_MIN_PROMPT_LENGTH = 1 | |||
There was a problem hiding this comment.
These are just placeholder values. Need input from LLM stakeholders to pick real values
nv-braf
commented
Oct 2, 2023
| @@ -36,62 +37,6 @@ | |||
| from model_analyzer.triton.server.server_config import TritonServerConfig | |||
|
|
|||
| from .config_command import ConfigCommand | |||
| from .config_defaults import ( | |||
There was a problem hiding this comment.
It was getting annoying (and ugly) to keep adding more defaults. So, I switched this to import all the defaults
nv-braf
commented
Oct 2, 2023
| @@ -71,23 +72,27 @@ def get_test_options(): | |||
| # The following options can be None: | |||
| # short_option | |||
| # expected_default_value | |||
| OptionStruct("int", "profile", "--client-max-retries", "-r", "125", "50"), | |||
There was a problem hiding this comment.
Boyscoutted the removal of magic numbers
nv-braf
force-pushed
the
new-cli-config-options-for-llm
branch
from
187c1c4
to
81d7e23
Compare
nv-braf
commented
Oct 2, 2023
| @@ -12,8 +12,8 @@ | |||
| # See the License for the specific language governing permissions and | |||
| # limitations under the License. | |||
|
|
|||
| ARG BASE_IMAGE=nvcr.io/nvidia/tritonserver:23.08-py3 | |||
There was a problem hiding this comment.
I don't know why these changes are showing up after I rebased, but these and the docs changes are safe to ignore.
There was a problem hiding this comment.
Maybe you can try this to remove the unwanted commit from PR: https://stackoverflow.com/a/51400593
There was a problem hiding this comment.
I'm not too concerned. They match what is on the add-llm-mode branch and will go away when this subbranch is pushed. Just wanted to call it out so you wouldn't review it.
nv-hwoo
reviewed
Oct 2, 2023
nv-hwoo
approved these changes
Oct 3, 2023
nv-braf
added a commit
that referenced
this pull request
Nov 3, 2023
* Adding new options for LLM (#768) * Update README and versions for 23.09 branch (#761) (#767) * Adding new options for LLM * Fixing codeQL issues * Fixing codeQL issue --------- Co-authored-by: Misha Chornyi <[email protected]> * Add LLM support to Brute Search (#769) * Initial coding complete * First unit test passing * Adding test for prompt length * Refactor PACG methods * Further refactoring * Ensure early exit isn't enabled for LLM models * Fix type checking errors * Attempt at fixing codeql issue * Revert "Attempt at fixing codeql issue" This reverts commit 2619b83. * Attempt at codeQL fix * Adding deepcopy back in * Removing deepcopy in an attempt to fix codeQL errors * Update model_analyzer/config/input/config_command_profile.py Co-authored-by: Hyunjae Woo <[email protected]> * Update model_analyzer/config/generate/perf_analyzer_config_generator.py Co-authored-by: Hyunjae Woo <[email protected]> * Update model_analyzer/config/generate/perf_analyzer_config_generator.py Co-authored-by: Hyunjae Woo <[email protected]> * Update model_analyzer/config/generate/perf_analyzer_config_generator.py Co-authored-by: Hyunjae Woo <[email protected]> * Moving location of method * Changing parameter to inference load * Changing parameter to inference load * Changing prompt length to text input length * Changing max_tokens to use request-parameter * Fix input-data typo * Changing non-parameter to parameter --------- Co-authored-by: Hyunjae Woo <[email protected]> * New LLM record types (#770) * New measurement fields created. * Fixing omission in llm_metric_table * Changing name to be avg_token_to_token... * New config options based on live run (#775) * Added new config options and modified existing options * Refactoring model parameter setting * Removing magic numbers * Capture LLM metrics from PA (#774) * Initial code for aggregation of new LLM metrics * New measurement fields created. * Fixing PA unit tests * Adding hooks in metrics to capture new LLM fields * Fixing codeQL errors * Fixing type checking errors * Changes needed post-merge from other branches * Revert naming mistake (due to merge). * Changes uncovered during live testing * Fixes based on hwoo review * Fixing typo * Change to use lists and mean() * Changes based on hwoo review * Correct how periodic concurrency works in PACG (#777) * Created a new class ConfigRangeNumeric and using it for periodic-concurrency * Fixes and defaults for periodic concurrency * First unit test passing * PACG chagnes complete. Unit tests updated and passing * Removing uneeded class * Fixing codeQL and hwoo's review suggestions * Adding missing else * Llm testing live run (#778) * Created a new class ConfigRangeNumeric and using it for periodic-concurrency * Fixes and defaults for periodic concurrency * First unit test passing * PACG chagnes complete. Unit tests updated and passing * Removing uneeded class * Changes to fix live run * Minor refactor and cleanup * Removing json files * Changing to use f-string * More cleanup from hwoo CR * Removing stale code for request period * Fix nit * Changes to get LLM summary reports working (#779) * Changes to get LLM summary reports working * Addressing hwoo's CR * Adding illegal LLM checks w/ unit testing + some minor cleanup (#781) * Adding illegal LLM checks w/ unit testing + some minor cleanup * Updated with TMA * Misc LLM cleanup (#782) * General cleanup * Add ticket nums to todos * Fix for non-LLM breaking bug introduced. * summary table in progress --------- Co-authored-by: Misha Chornyi <[email protected]> Co-authored-by: Hyunjae Woo <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added the following new options to support LLM: