fix: consistent env var parsing for all boolean args in vLLM #98
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.
Support consistent parsing of boolean arguments defined in vLLM.
Description
In vLLM, some args use action='store_true':
https://github.com/vllm-project/vllm/blob/df845b2b46c3e30f5bd3e3be286285ed148323fc/vllm/engine/arg_utils.py#L202-L204
and
--enable-chunked-prefill
has a custom arg parsing action calledStoreBoolean
:https://github.com/vllm-project/vllm/blob/df845b2b46c3e30f5bd3e3be286285ed148323fc/vllm/engine/arg_utils.py#L561-L568
We want
EnvVarArgumentParser
to treat all of these boolean args consistently. The fix in this PR is to inspect the type of the action in addition toaction.type
to detect when we need to parse a boolean from the env var value.In my testing, I also learned that
argparse
will parse default string values into the correct type for the argument, so there's no need to have special handling forint
args (which would also imply special handling offloat
,_bool_from_string
,nullable_str
, and other typed args).How Has This Been Tested?
Tested by hand for the actual vLLM args and also added unit tests of
EnvVarArgumentParser
for the'store_true'
,'store_false'
, andStoreBoolean
action types for boolean args.Merge criteria: