[shenoyvvarun]

Motivation

• When I send a request without any max_tokens or max_completion_tokens. Sglang crashes because of this code.

  File "/tmp/vasheno_sg/python/sglang/srt/managers/schedule_batch.py", line 657, in adjust_max_prefix_ids
    if self.sampling_params.max_new_tokens > 0:
TypeError: '>' not supported between instances of 'NoneType' and 'int'

Modifications

3 options for fixing this:

1. Chosen Add a default value check for max_new_tokens in SamplingParams constructor.
2. Remove None values during construction of sampling_params kwargs here .
3. None check before using the max_new_tokens here

Accuracy Test

Benchmark & Profiling

Checklist

• Format your code according to the Code Formatting with Pre-Commit.
• Add unit tests as outlined in the Running Unit Tests.
• Update documentation / docstrings / example tutorials as needed, according to Writing Documentation.
• Provide throughput / latency benchmark results and accuracy evaluation results as needed, according to Benchmark and Profiling and Accuracy Results.
• For reviewers: If you haven't made any contributions to this PR and are only assisting with merging the main branch, please remove yourself as a co-author when merging the PR.
• Please feel free to join our Slack channel at https://slack.sglang.ai to discuss your PR.

[gemini-code-assist]

Summary of Changes

Hello @shenoyvvarun, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

I've implemented a bug fix to prevent a runtime crash in Sglang that occurred when max_new_tokens was not explicitly provided in a request, leading to it being None. The change ensures that the SamplingParams object always initializes max_new_tokens with a valid integer value, defaulting to 128 if None is passed, thereby resolving the TypeError.

Highlights

• max_new_tokens Bugfix: I've addressed a TypeError that occurred when max_new_tokens was None in SamplingParams. The fix involves setting self.max_new_tokens to a default value of 128 if the input max_new_tokens is None, otherwise using the provided value. This prevents crashes when no max_tokens or max_completion_tokens are specified in requests.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request fixes a TypeError that occurs when max_new_tokens is None in SamplingParams. The crash happens because of an attempt to compare None with an integer. The proposed fix is to provide a default value for max_new_tokens within the SamplingParams constructor if it is None.

The fix is correct and effectively resolves the crash. I have one suggestion regarding code maintainability to avoid duplicating the default value. Overall, this is a good fix.

[gemini-code-assist]

This correctly handles the case where max_new_tokens is None, fixing the crash. However, this change hardcodes the default value 128, which is already specified as the default in the __init__ method signature on line 33. This duplication can lead to inconsistencies if the default value is changed in one place but not the other. Consider refactoring to use a single source for this default value, for instance by defining a constant at the module or class level and using it in both places.

[shenoyvvarun]

Based on preference, I can remove the default value assignment for parameter max_new_tokens

[CatherineSue]

I think adding the change here could be a good band-aid solution for the fix.

For long term, maybe we should dynamically determine the default max_new_tokens (if it is None) by checking the model's context length minus the input length. Maybe add this in verify?

Could you also add a UT for this?

[CatherineSue]

cc @slin1237

[hnyls2002]

Thanks @shenoyvvarun! max_new_tokens now defaults and is None-guarded on main, so this no longer applies. closing. Please reopen if it still misbehaves.