[Bug]: quantization cli choice not setup after SkipValidation

[Yikun]

Your current environment

vllm main

🐛 Describe the bug

This is a issue relate to #18843

Issue

vllm ascend has a quantization extension to extend quantization choice:
https://github.com/vllm-project/vllm-ascend/blob/92bc5576d8899cff4e041e20af11a4f1d46aa066/vllm_ascend/platform.py#L76-L80

But after 3c49dbd , the quantization choices return None.

The problem are found in: vllm-project/vllm-ascend#1042

  File "/__w/vllm-ascend/vllm-ascend/vllm_ascend/platform.py", line 79, in pre_register_and_update
    if ASCEND_QUATIZATION_METHOD not in quant_action.choices:
TypeError: argument of type 'NoneType' is not iterable

Invisitigation:

vllm/vllm/engine/arg_utils.py

Line 449 in 432ec99

model_kwargs = get_kwargs(ModelConfig)

• Expected (Without SkipValidation):

  type_hints
  {<class 'NoneType'>, typing.Literal['aqlm', 'awq', 'deepspeedfp', 'tpu_int8', 'fp8', 'ptpc_fp8', 'fbgemm_fp8', 'modelopt', 'modelopt_fp4', 'marlin', 'bitblas', 'gguf', 'gptq_marlin_24', 'gptq_marlin', 'gptq_bitblas', 'awq_marlin', 'gptq', 'compressed-tensors', 'bitsandbytes', 'qqq', 'hqq', 'experts_int8', 'neuron_quant', 'ipex', 'quark', 'moe_wna16', 'torchao', 'auto-round']}
  

  vllm/vllm/engine/arg_utils.py

  Lines 205 to 206 in 432ec99

  	elif contains_type(type_hints, Literal):
  	kwargs[name].update(literal_to_kwargs(type_hints))

  
  and finally set choice succesfully:
  https://github.com/vllm-project/vllm/blob/main/vllm/engine/arg_utils.py#L136-L145

• Unexpected (With SkipValidation):
  But after add SkipValidation:
  

  vllm/vllm/engine/arg_utils.py

  Lines 239 to 240 in 432ec99

  	or any(is_not_builtin(th) for th in type_hints)):
  	kwargs[name]["type"] = str

  type_hints
  {typing.Optional[typing.Literal['aqlm', 'awq', 'deepspeedfp', 'tpu_int8', 'fp8', 'ptpc_fp8', 'fbgemm_fp8', 'modelopt', 'modelopt_fp4', 'marlin', 'bitblas', 'gguf', 'gptq_marlin_24', 'gptq_marlin', 'gptq_bitblas', 'awq_marlin', 'gptq', 'compressed-tensors', 'bitsandbytes', 'qqq', 'hqq', 'experts_int8', 'neuron_quant', 'ipex', 'quark', 'moe_wna16', 'torchao', 'auto-round']]}
  

  So, the choice of quantization is not set as expected

Workaround:

I try to fix it in vllm, it works

diff --git a/vllm/engine/arg_utils.py b/vllm/engine/arg_utils.py
index 555532526..354452ff7 100644
--- a/vllm/engine/arg_utils.py
+++ b/vllm/engine/arg_utils.py
@@ -476,6 +476,7 @@ class EngineArgs:
         model_group.add_argument("--max-model-len",
                                  **model_kwargs["max_model_len"])
         model_group.add_argument("--quantization", "-q",
+                                 choices=list(get_args(QuantizationMethods)),
                                  **model_kwargs["quantization"])
         model_group.add_argument("--enforce-eager",
                                  **model_kwargs["enforce_eager"])

But I think it should have a better way to setup choice by using typing to resolve it, I am not very familar with it.

Could you give some suggestion? Maybe cc @hmellor

Before submitting a new issue...

• Make sure you already searched for relevant issues, and asked the chatbot living at the bottom right corner of the documentation page, which can answer lots of frequently asked questions.

[hmellor]

We should change line ~159 so that it can handle Annotated (can come from SkipValidation) and Union (can come from Optional) at the same time.

Right now it cannot handle both at the same time, which is why you are experiencing problems.

[Yikun]

Right now it cannot handle both at the same time, which is why you are experiencing problems.

Ah, sounds like we need a recursion way to travel all case. like:

diff --git a/vllm/engine/arg_utils.py b/vllm/engine/arg_utils.py
index 555532526..01b1888df 100644
--- a/vllm/engine/arg_utils.py
+++ b/vllm/engine/arg_utils.py
@@ -149,18 +149,29 @@ def is_not_builtin(type_hint: TypeHint) -> bool:
     """Check if the class is not a built-in type."""
     return type_hint.__module__ != "builtins"

+def _get_valid_type_hints(field_type) -> set:
+    type_hints = set()
+    origin = get_origin(field_type)
+
+    if origin is Annotated:
+        annotated_args = get_args(field_type)
+        type_hints.update(get_valid_type_hints(annotated_args[0]))
+    elif origin is Union:
+        for arg in get_args(field_type):
+            if not isinstance(arg, SkipValidation):
+                type_hints.update(get_valid_type_hints(arg))
+    else:
+        if not isinstance(field_type, SkipValidation):
+            type_hints.add(field_type)
+
+    return type_hints

 def get_kwargs(cls: ConfigType) -> dict[str, Any]:
     cls_docs = get_attr_docs(cls)
     kwargs = {}
     for field in fields(cls):
         # Get the set of possible types for the field
-        type_hints: set[TypeHint] = set()
-        if get_origin(field.type) in {Union, Annotated}:
-            predicate = lambda arg: not isinstance(arg, SkipValidation)
-            type_hints.update(filter(predicate, get_args(field.type)))
-        else:
-            type_hints.add(field.type)
+        type_hints: set[TypeHint] = _get_valid_type_hints(field.type)

         # If the field is a dataclass, we can use the model_validate_json
         generator = (th for th in type_hints if is_dataclass(th))

To be honest, I'm a bit new to type systems and get_kwargs is also little bit complex to me, lol. Would you mind submitting the PR? I can help to test it.

[hmellor]

We could probably use something like a while loop if we don't want recursion.

Yeah I can make a PR tomorrow when I'm back at my computer.

[lengrongfu]

+def _extract_types_recursive(type_hint: TypeHint) -> set[TypeHint]:
+    result = set()
+
+    if get_origin(type_hint) in {Union, Annotated}:
+        args = get_args(type_hint)
+        for arg in args:
+            if isinstance(arg, SkipValidation):
+                continue
+            # Recursively process nested Union/Annotated types
+            result.update(_extract_types_recursive(arg))
+    else:
+        result.add(type_hint)
+
+    return result
+
+
 def get_kwargs(cls: ConfigType) -> dict[str, Any]:
     cls_docs = get_attr_docs(cls)
     kwargs = {}
     for field in fields(cls):
-        # Get the set of possible types for the field
-        type_hints: set[TypeHint] = set()
-        if get_origin(field.type) in {Union, Annotated}:
-            predicate = lambda arg: not isinstance(arg, SkipValidation)
-            type_hints.update(filter(predicate, get_args(field.type)))
-        else:
-            type_hints.add(field.type)
+        type_hints: set[TypeHint] = _extract_types_recursive(field.type)

Is this what you want? @hmellor

[hmellor]

I've just made #19093 with a fix (I made you a co-author @Yikun as the solution is heavily inspired by your comment)

[Yikun]

Thanks all @hmellor @lengrongfu

Closed by #19093, merged to main.