-
Notifications
You must be signed in to change notification settings - Fork 27
fix: Salt failure is a hard error #406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
9e6989d
ba6bb2f
6cdbe52
de68f37
a328bca
74a0621
44fc32d
ae02652
6f6c165
e7e1008
a72e50d
83a61d5
659debf
6d28c00
cbaa2c3
93982be
a6e284f
d7191bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,7 +93,7 @@ settings: | |
| warmup: | ||
| enabled: false # Enable warmup phase before performance run | ||
| n_requests: null # Warmup request count (None = full dataset once) | ||
| salt: true # Prepend a unique random hex salt to each warmup prompt | ||
| salt: false # Prepend a unique random hex salt to each warmup prompt. Requires text-'prompt' samples; enabling it on a pre-tokenized ('input_tokens') dataset is a hard error. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing the default behavior to false might fail a lot of the submission silently (simply because the submitter forgot to turn them on).
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By default, both warmup and warmup salting are disabled. Enabling warmup alone leaves salting disabled; the run logs a warning and proceeds with unsalted warmup requests. A hard setup error occurs only when both warmup and salt are enabled and the loaded dataset contains a sample the implementation cannot safely salt. The bug this PR addresses is that, previously, requesting salt could still result in an unsalted outgoing request—sometimes silently and sometimes with only a warning.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that part makes sense. But for submission, only a warmup with SALT will ever be accepted for single-turn/agentic LLM workload. Will "SALT by default enabled" work? Or can a ruleset force that to be true? Thinking about reducing human mistakes and amount of context knowledge here This is given that all future LLM/VLM workload will use text prompt whenever possible
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree on reducing the potential for mistakes. For submission we should only allow warmup with salting, but that can change if we want to benchmark a scenario where all the requests share the same prefix (system prompt) and want to capture the performance in that scenario. This will be captured in the benchmark specification via rules and enforced by the system. As we move towards task specific datasets, the dataset will encode how to salt it given a random seed, and that will be more robust instead of relying on a general salting mechanism. So, the scenario above will be handled by either salting the system prompt (fill cache invalidation) or salting the user prompt (system prompt cached, user prompt invalidated). |
||
| drain: false # Drain in-flight warmup requests before starting the performance phase | ||
| warmup_random_seed: 42 # RNG seed for warmup scheduling and sample ordering | ||
| profiling: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.