[https://nvbugs/6141606][fix] Move the layer_types derivation into Qwen3HybridConfig.from_hf (where `pretr#13832
Conversation
📝 WalkthroughWalkthrough
ChangesQwen3HybridConfig Derivation Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tensorrt_llm/bench/build/dataclasses.py (1)
275-276: ⚡ Quick winAdd explicit type annotations to
from_hf.This modified method should annotate inputs and return type for mypy/static checks.
Proposed fix
`@classmethod` - def from_hf(cls, model_hf_name, hf_model_path): + def from_hf(cls, model_hf_name: str, hf_model_path: str | None) -> "Qwen3HybridConfig":As per coding guidelines, "Always annotate functions; make the return type
Noneif the function does not return anything".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tensorrt_llm/bench/build/dataclasses.py` around lines 275 - 276, Annotate the classmethod signature for from_hf by adding explicit types: change def from_hf(cls, model_hf_name, hf_model_path): to def from_hf(cls, model_hf_name: str, hf_model_path: Optional[str]) -> Self (or -> "YourClassName" if not using Python 3.11), and add the required imports (from typing import Optional and, if available, Self; otherwise import typing and use typing.Any or the concrete class name). Ensure the annotation matches other dataclasses in this module and that mypy/static checks will accept the return type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tensorrt_llm/bench/build/dataclasses.py`:
- Around line 275-276: Annotate the classmethod signature for from_hf by adding
explicit types: change def from_hf(cls, model_hf_name, hf_model_path): to def
from_hf(cls, model_hf_name: str, hf_model_path: Optional[str]) -> Self (or ->
"YourClassName" if not using Python 3.11), and add the required imports (from
typing import Optional and, if available, Self; otherwise import typing and use
typing.Any or the concrete class name). Ensure the annotation matches other
dataclasses in this module and that mypy/static checks will accept the return
type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 34a2b780-3527-4677-abcb-e9bb987f2219
📒 Files selected for processing (1)
tensorrt_llm/bench/build/dataclasses.py
|
lgtm! |
…aded config The Qwen3HybridConfig.set_values_if_none validator called load_pretrained_config(self.name, ...) where self.name is the bench alias (e.g. qwen3.5_9b_hf), not an actual path. Transformers then tried to resolve it as a hub repo id and failed with a 401/OSError. Move the layer_types derivation into from_hf, where the pretrained_config is already loaded from the correct hf_model_path, and pass the derived num_attention_layers/num_linear_attention_layers through the constructor. Signed-off-by: tensorrt-cicd <[email protected]>
68ad06d to
353569d
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #49637 [ run ] triggered by Bot. Commit: |
|
PR_Github #49637 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #49817 [ run ] triggered by Bot. Commit: |
|
PR_Github #49817 [ run ] completed with state |
…`Qwen3HybridConfig.from_hf` (where `pretr (NVIDIA#13832) Signed-off-by: tensorrt-cicd <[email protected]>
…`Qwen3HybridConfig.from_hf` (where `pretr (NVIDIA#13832) Signed-off-by: tensorrt-cicd <[email protected]>
Summary
set_values_if_nonecalledload_pretrained_config(self.name, ...)whereself.nameis the bench alias (e.g.qwen3.5_9b_hf), not the checkpoint path, so transformers tried to resolve it as an HF hub repo id and failed with 401/OSError.layer_typesderivation intoQwen3HybridConfig.from_hf(wherepretrained_configis already correctly loaded fromhf_model_path) and pass the resultingnum_attention_layers/num_linear_attention_layersthrough the constructor, so the validator never needs to re-resolve by name.Test plan
Links
Summary by CodeRabbit