[https://nvbugs/6327149][fix] Handle EXAONE 4.5 33B memory constraints - #16992
[https://nvbugs/6327149][fix] Handle EXAONE 4.5 33B memory constraints#16992yechank-nvidia wants to merge 1 commit into
Conversation
|
/bot run |
Signed-off-by: yechank <161688079+yechank-nvidia@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughEXAONE 4.5 configuration handling now normalizes compatible trailing MTP layer types before validation, preserves errors for mismatched structures, and gates subsequent multimodal accuracy tests on available device memory. ChangesEXAONE 4.5 MTP configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant TextConfigInput
participant Exaone4_5Config
participant HuggingFaceConfig
TextConfigInput->>Exaone4_5Config: pass text_config dictionary
Exaone4_5Config->>Exaone4_5Config: normalize compatible trailing MTP layer_types
Exaone4_5Config->>HuggingFaceConfig: initialize with normalized text_config
HuggingFaceConfig-->>Exaone4_5Config: validate configuration or raise mismatch error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tensorrt_llm/_torch/models/modeling_exaone4_5.py (1)
41-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComplete the new Python 3.10 type contracts.
Both new function signatures use incomplete or legacy annotations, contrary to the repository’s required typing style.
tensorrt_llm/_torch/models/modeling_exaone4_5.py#L41-L64: replace baredictwith a parameterized mutable mapping type.tensorrt_llm/_torch/models/modeling_exaone4_5.py#L118-L131: use|unions, parameterized dictionaries, annotate**kwargs, and add-> None.As per coding guidelines, “Annotate every function” and “prefer built-in generic types and
|.”🤖 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/_torch/models/modeling_exaone4_5.py` around lines 41 - 64, Complete the Python 3.10 annotations in _normalize_exaone4_5_mtp_layer_types at tensorrt_llm/_torch/models/modeling_exaone4_5.py:41-64 by replacing bare dict with a parameterized mutable mapping type. Also update the function at tensorrt_llm/_torch/models/modeling_exaone4_5.py:118-131 to use | unions, parameterized dictionaries, an annotation for **kwargs, and -> None, following the repository’s built-in generic typing style.Source: Coding guidelines
🤖 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/_torch/models/modeling_exaone4_5.py`:
- Around line 41-64: Complete the Python 3.10 annotations in
_normalize_exaone4_5_mtp_layer_types at
tensorrt_llm/_torch/models/modeling_exaone4_5.py:41-64 by replacing bare dict
with a parameterized mutable mapping type. Also update the function at
tensorrt_llm/_torch/models/modeling_exaone4_5.py:118-131 to use | unions,
parameterized dictionaries, an annotation for **kwargs, and -> None, following
the repository’s built-in generic typing style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 98df765b-8b2e-4dee-8d5f-d3e35ca4fbef
📒 Files selected for processing (3)
tensorrt_llm/_torch/models/modeling_exaone4_5.pytests/integration/defs/accuracy/test_llm_api_pytorch_multimodal.pytests/unittest/_torch/modeling/test_modeling_exaone4_5.py
d600e86 to
85ff940
Compare
|
PR_Github #62417 [ run ] triggered by Bot. Commit: |
|
/bot run |
|
PR_Github #62420 [ run ] triggered by Bot. Commit: |
|
PR_Github #62417 [ run ] completed with state |
|
PR_Github #62420 [ run ] completed with state
|
|
Holding on one thing in the new Today it's inert — The problem shows up whenever transformers is bumped to >=5.8. On that path you subclass HF's config as Cheapest fixes: name the subclass Separately, worth stating explicitly in the description: |
| return | ||
|
|
||
| logger.warning( | ||
| "EXAONE 4.5 config includes %d trailing MTP layer type(s); " |
There was a problem hiding this comment.
Nit: our logger isn't the python standard logger, so use f-strings instead of printf style formatting.
| super().__init__(text_config=text_config, vision_config=vision_config, **kwargs) | ||
|
|
||
| AutoConfig.register(Exaone4_5Config.model_type, Exaone4_5Config, exist_ok=True) | ||
| else: |
There was a problem hiding this comment.
Nit: is there a reason to have this in the else statement rather than in the try block? Usually else is useful if we don't want the code in that block to have the exception type(s) caught (in this case, ImportError).
| @@ -1,12 +1,14 @@ | |||
| # SPDX-FileCopyrightText: Copyright (c) 2022-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |||
There was a problem hiding this comment.
It looks like this file isn't scheduled to run anywhere, can you please add it to the CI test-db or QA test lists?
5 participants
Dev Engineer Review
layer_typesinclude extra trailing MTP-only entries, while keeping strict validation for unexpectedlayer_typescount mismatches._normalize_exaone4_5_mtp_layer_types(text_config: dict)and wired it into both the local (HF import fallback) and native HF config paths via a defensivecopy.deepcopywhentext_configis provided as a dict; logs a warning and truncates only for the compatible_num_mtp_layers == num_nextn_predict_layerscase.Exaone4_5Configto alias the native HFExaone4_5_Configas_HfExaone4_5Configand wrap/extend it to apply the legacy FP8 MTP layout compatibility.@pytest.mark.skip_less_device_memory(60000)onTestExaone4_5_33Bto address device memory constraints.tests/integration/test_lists/waives.txtforTestExaone4_5_33B::test_auto_dtype[...]parameterizationsforced_chunked_prefillandfull_budget.QA Engineer Review
Tests modified (code):
tests/unittest/_torch/modeling/test_modeling_exaone4_5.pytest_exaone4_5_config_normalizes_trailing_mtp_layer_typestest_exaone4_5_config_preserves_unexpected_layer_type_mismatchtests/integration/defs/accuracy/test_llm_api_pytorch_multimodal.py@pytest.mark.skip_less_device_memory(60000)immediately beforeTestExaone4_5_33BTest-list / CI waiver modified:
tests/integration/test_lists/waives.txtTestExaone4_5_33B::test_auto_dtype[...]coveringforced_chunked_prefillandfull_budgetunder thefull:L40S/accuracy/test_llm_api_pytorch_multimodal.pysection.Coverage in
tests/integration/test_lists/:tests/integration/test_lists/test-db/ortests/integration/test_lists/qa/(not referenced there).Verdict: needs follow-up.