Skip to content

Conversation

sjmonson
Copy link
Collaborator

@sjmonson sjmonson commented Sep 23, 2025

Summary

Details

  • [ ]

Test Plan

Related Issues

  • Resolves #

  • "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 ##)

@sjmonson sjmonson changed the base branch from main to features/refactor/working September 23, 2025 15:00
@sjmonson sjmonson changed the title [GuideLLM Refactor] scenarios reenablement [GuideLLM Refactor] Scenarios reenablement Sep 23, 2025
@sjmonson sjmonson requested review from markurtz and jaredoconnell and removed request for markurtz September 23, 2025 18:04
sjmonson added a commit that referenced this pull request Sep 23, 2025
…e-draft

[GuideLLM Refactor] Scenarios reenablement #362
Copy link
Collaborator

@jaredoconnell jaredoconnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to break the non-scenario pathway. And I'm running into issues running the built-in scenario.

Regarding the issues with the built-in profile:
The built-in scenario rag appears to use the old name for the profile, rate_type. The result of this is the command line still requesting that I specify the profile type despite it being included in the profile.

There is also an error trying to use the rag profile:

  File "/Users/joconnel/Documents/projects/ai/guidellm/src/guidellm/dataset/creator.py", line 94, in create
    dataset = cls.handle_create(
        data, data_args, processor, processor_args, random_seed
    )
  File "/Users/joconnel/Documents/projects/ai/guidellm/src/guidellm/dataset/in_memory.py", line 42, in handle_create
    data_dict = cls.format_data_dict(data)
  File "/Users/joconnel/Documents/projects/ai/guidellm/src/guidellm/dataset/in_memory.py", line 64, in format_data_dict
    raise TypeError(
    ...<2 lines>...
    )
TypeError: Unsupported data format. Expected Dict[str, Iterable[Any]], got <class 'dict'>

Are the built in scenarios up to date?

Regarding the issue with the non-scenario pathway, it's sending a list of integers to the profiles that need an integer, causing a validation failure:
guidellm benchmark run --target http://localhost:8000 --data "prompt_tokens=256,output_tokens=128" --rate-type constant --rate 2 --max-seconds 10

TypeError: Unable to apply constraint 'gt' to supplied value [2.0]

It's possible that some of this is user error, but if that's the case the input validation is not doing a good job.

@sjmonson sjmonson force-pushed the features/refactor/scenarios branch 2 times, most recently from ea26181 to fb5c153 Compare September 24, 2025 18:50
@sjmonson
Copy link
Collaborator Author

sjmonson commented Sep 24, 2025

It turns out the built-in scenarios have been broken for a long while. I don't love how this feature works at the moment and want to rethink it in the next update. For now this provides basic reenablement in the refactor.

@sjmonson sjmonson force-pushed the features/refactor/scenarios branch from fb5c153 to d57fe3d Compare September 24, 2025 18:58
sjmonson added a commit that referenced this pull request Sep 25, 2025
@jaredoconnell jaredoconnell force-pushed the features/refactor/scenarios branch from 1e47d98 to 68e9f55 Compare September 25, 2025 20:01
Copy link
Collaborator

@jaredoconnell jaredoconnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. With the fix I pushed it works for me, though I haven't had the time to let a full run complete.
I have one comment, but I won't block on that. I am also willing to do that work for you if you like the idea.

sjmonson and others added 5 commits September 25, 2025 17:20
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]>
@jaredoconnell jaredoconnell force-pushed the features/refactor/scenarios branch from 68e9f55 to 03f9085 Compare September 25, 2025 21:22
Base automatically changed from features/refactor/working to features/refactor/base September 29, 2025 16:37
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the scenario system in GuideLLM to enable scenarios functionality. The changes transition from a nested JSON structure to a flat string-based data format and introduce comprehensive scenario support across the codebase.

  • Simplifies scenario JSON configuration files to use flat string data format instead of nested objects
  • Adds comprehensive scenario support with validation, file loading, and decorator functionality
  • Integrates scenarios into the CLI with builtin scenario selection and file-based configuration

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/guidellm/benchmark/scenarios/rag.json Converts nested data structure to flat string format
src/guidellm/benchmark/scenarios/chat.json Converts nested data structure to flat string format
src/guidellm/benchmark/scenario.py Adds scenario loading, validation, and decorator functionality
src/guidellm/benchmark/profile.py Updates type annotations and removes redundant type checks
src/guidellm/benchmark/entrypoints.py Adds scenarios decorator to benchmark function
src/guidellm/benchmark/benchmarker.py Fixes constraints parameter handling
src/guidellm/benchmark/init.py Exports new scenario-related symbols
src/guidellm/main.py Integrates scenario support into CLI with validation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@jaredoconnell jaredoconnell force-pushed the features/refactor/scenarios branch from bb74a55 to 2c6637d Compare September 29, 2025 21:28
sjmonson added a commit that referenced this pull request Sep 30, 2025
@sjmonson sjmonson merged commit 27d5702 into features/refactor/base Sep 30, 2025
6 of 17 checks passed
@sjmonson sjmonson deleted the features/refactor/scenarios branch September 30, 2025 15:36
@markurtz markurtz added this to the v0.4.0 milestone Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants