Validate positive inference min output tokens - #8370
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed1163963d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """ | ||
|
|
||
| min_out_tokens: int = Field(1, alias="min_tokens") | ||
| min_out_tokens: int = Field(1, alias="min_tokens", gt=0) |
There was a problem hiding this comment.
This is a non-merge commit, but its commit message contains no Signed-off-by trailer, so it violates the repository's mandatory commit/CI requirement and may be rejected by the corresponding compliance check. Recreate the commit with --signoff using the configured Git identity.
AGENTS.md reference: AGENTS.md:L6-L8
Useful? React with 👍 / 👎.
min_out_tokens has no lower-bound validation, so a negative or zero value passes config construction and is only caught later inside GenWorkSpace's C++ workspace allocation. Add the same gt=0 constraint deepspeedai#8343 gave the sibling max_out_tokens field. Refs deepspeedai#8339 Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
ed11639 to
e5542cf
Compare
1 participant
Description
DeepSpeedInferenceConfig.min_out_tokenshas no lower-bound validation, so a negative orzero value passes Pydantic construction and is only ever caught later, deep in
GenWorkSpace's C++ workspace allocation (inference_context.h), which takes it as anunsigned parameter. #8343 added the same
gt=0constraint to the architecturallyidentical sibling field
max_out_tokens; this completes it onmin_out_tokens, whichthat PR does not touch.
Refs #8339 (reports the
max_out_tokenshalf of this gap, already being fixed by #8343).Testing
Verified with a plain Pydantic construction check, no model or GPU required:
DeepSpeedInferenceConfig(min_out_tokens=-1)andDeepSpeedInferenceConfig(min_out_tokens=0)both construct without error.ValidationError, andmin_out_tokens=50(or the default1) still constructs normally.tests/unit/inference/test_inference_config.py(mirrors Validate positive inference max output tokens #8343's shape formax_out_tokens) fails on unmodifiedmasterand passes on this branch; ran withpytest -m inference tests/unit/inference/test_inference_config.py.pre-commit run --files deepspeed/inference/config.py tests/unit/inference/test_inference_config.py: all hooks pass.GenWorkSpacepath itself, since this VPS has no GPU. The C++ guard atinference_context.h:150is untouched and stays as a defense in depth; this PR only stops the invalid value earlier, at config time.