Base/staging branch for GuideLLM refactor for all PRs to eventually be merged into before landing on main #351
Conversation
This file contains hidden or 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
… refactor branch Signed-off-by: Mark Kurtz <[email protected]>
markurtz
force-pushed
the
features/refactor/base
branch
from
ebdb6ce to
a2d19cd
Compare
… refactor branch Signed-off-by: Mark Kurtz <[email protected]>
…ng config.py to settings.py due to later config additions and potential conflicts in naming Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
…view Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
… for plural Signed-off-by: Mark Kurtz <[email protected]>
…p to avoid conflicts Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: Mark Kurtz <[email protected]>
Signed-off-by: jaredoconnell <[email protected]>
## Summary <!-- Include a short paragraph of the changes introduced in this PR. If this PR requires additional context or rationale, explain why the changes are necessary. --> TODO --- - [x] "I certify that all code in this PR is my own, except as noted below." ## Use of AI - [ ] Includes AI-assisted code completion - [ ] Includes code generated by an AI application - [ ] Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes `## WRITTEN BY AI ##`)
## Summary This PR ports the new functionality from `benchmark run` to `benchmark from-file`, and does so in a way that reuses as much code as practical to have one source of truth. ## Details <!-- Provide a detailed list of all changes introduced in this pull request. --> - Fixes from-file by making it to use the new output format. - Moves code related to the new output formats to separate functions that are called from both benchmark entrypoints. - Moves additional chunks of code out of the large benchmark run entrypoint function for modularity. ## Test Plan Run a benchmark with an output of json or yaml, and use `from-file` to re-import it and export it. You can select any output type supported by `benchmark run`. `guidellm benchmark from-file ./result.json --output-formats console` `guidellm benchmark from-file ./result.yaml --output-formats yaml` ## Related Issues --- - [x] "I certify that all code in this PR is my own, except as noted below." ## Use of AI - [x] Includes AI-assisted code completion - [ ] Includes code generated by an AI application - [ ] Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes `## WRITTEN BY AI ##`) --------- Signed-off-by: Jared O'Connell <[email protected]>
## Summary Reintroduces a few changes from main --------- Signed-off-by: Samuel Monson <[email protected]>
Replace scenario entrypoint with a decorator Forward-port get_default and from_file to Scenario Apply scenario args as an update to kwargs Readd scenario support to CLI Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Jared O'Connell <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Co-authored-by: Samuel Monson <[email protected]> Signed-off-by: Jared O'Connell <[email protected]>
## Summary This PR gets the CSV output to a state comparable to pre-refactor. ## Details Implements the functions to export the required data to the CSV format. The goal is to include the required information in the CSV without cluttering it, but also without creating too much of a burden to the future maintainers resulting from referencing specific schema elements. The following columns are new: - Profile (an entire JSON dump of the profile) - Backend (the entire JSON dump of the internal data structure) - Generator Data You can view these files in the attached output generated by the following test: `guidellm benchmark run --output-path result_7.csv --max-seconds 2 --target=http://localhost:8000 --data "prompt_tokens=256,output_tokens=128" --rate-type constant --rate 1 --output-formats csv` [result_7.csv](https://github.com/user-attachments/files/22222220/result_7.csv) ## Test Plan Run GuideLLM with the following additional args: `--output-path result.csv --output-formats csv` The generated file should have all info required. ## Related Issues This is a part of the scheduler refactor. --- - [x] "I certify that all code in this PR is my own, except as noted below." ## Use of AI - [ ] Includes AI-assisted code completion - [ ] Includes code generated by an AI application - [ ] Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes `## WRITTEN BY AI ##`)
## Summary <!-- Include a short paragraph of the changes introduced in this PR. If this PR requires additional context or rationale, explain why the changes are necessary. --> Bumps the minimum python version to 3.10 and updates all dependent tooling (CI, tox, ruff, mypy). --- - [x] "I certify that all code in this PR is my own, except as noted below." ## Use of AI - [ ] Includes AI-assisted code completion - [ ] Includes code generated by an AI application - [ ] Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes `## WRITTEN BY AI ##`)
<!-- Include a short paragraph of the changes introduced in this PR. If this PR requires additional context or rationale, explain why the changes are necessary. --> Fixes a issue in metric calculation that caused incorrect statistics at extreme changes in concurrency and an issue where the first decode token was not counted in total tokens per second. <!-- Provide a detailed list of all changes introduced in this pull request. --> - [x] Fixed issue where merged concurrency change events would double-count concurrency - [x] Ensure first decode token is counted when calculating total tokens per second <!-- List the steps needed to test this PR. --> - Run unit tests: `tox -e test-unit -- -m "regression and sanity"` --- - [x] "I certify that all code in this PR is my own, except as noted below." - [x] Includes AI-assisted code completion - [ ] Includes code generated by an AI application - [x] Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes `## WRITTEN BY AI ##`) --------- Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
## Summary <!-- Include a short paragraph of the changes introduced in this PR. If this PR requires additional context or rationale, explain why the changes are necessary. --> Fixes failing unit tests. Most were failing due to changes in functionality but a couple were regressions. --- - [x] "I certify that all code in this PR is my own, except as noted below." ## Use of AI - [x] Includes AI-assisted code completion - [ ] Includes code generated by an AI application - [x] Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes `## WRITTEN BY AI ##`)
## Summary This PR fixes all type errors in the utils package. Only a few were ignored. ## Details - A lot of these changes are reflecting that values can be None, and the associated None checks. - Others are incorrect type annotations - Others are asserting with cast that we know for certain that the type is correct. - Plus other minor changes ## Test Plan Run the tests and look through the changes to make sure the logic is equivalent or better to the original code. --- - [x] "I certify that all code in this PR is my own, except as noted below." ## Use of AI - [x] Includes AI-assisted code completion - [ ] Includes code generated by an AI application - [ ] Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes `## WRITTEN BY AI ##`)
Many of the quality errors are due to using the older union style, and have appeared due to the upgrade of the minimum Python version from 3.9 to 3.10 Signed-off-by: Jared O'Connell <[email protected]>
Signed-off-by: Jared O'Connell <[email protected]>
Signed-off-by: Jared O'Connell <[email protected]>
Signed-off-by: Jared O'Connell <[email protected]>
## Summary This fixes quality errors in the code. ## Details The recent switch to Python 3.10 means that the linter can apply 3.10 specific linters, so this fixes errors that occur now that backwards compatibility is no longer required. The biggest change is the switch from explicit `Union` references to `|`. I also took advantage of the version change to use `match`. Also, it appears that the `from __future__ import annotations` lines are masking a lot of circular imports. So I can't remove those yet. ## Test Plan Run GuideLLM as normal, make sure the tests pass, and confirm that there are no more quality errors. --- - [x] "I certify that all code in this PR is my own, except as noted below." ## Use of AI - [x] Includes AI-assisted code completion - [ ] Includes code generated by an AI application - [x] Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes `## WRITTEN BY AI ##`)
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Jared O'Connell <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
## Summary <!-- Include a short paragraph of the changes introduced in this PR. If this PR requires additional context or rationale, explain why the changes are necessary. --> Default to marking tests that timeout as XFAIL. ## Details <!-- Provide a detailed list of all changes introduced in this pull request. --> - [ ] ## Test Plan <!-- List the steps needed to test this PR. --> - ## Related Issues <!-- Link any relevant issues that this PR addresses. --> - Closes #404 --- - [x] "I certify that all code in this PR is my own, except as noted below." ## Use of AI - [ ] Includes AI-assisted code completion - [ ] Includes code generated by an AI application - [x] Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes `## WRITTEN BY AI ##`)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
TODO
Details
TODO
Test Plan
TODO
Related Issues
TODO