[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 |
WalkthroughEXAONE 4.5 configuration handling normalizes compatible trailing MTP layer types before sub-configuration validation and preserves errors for mismatched structures. Multimodal accuracy tests now require sufficient device memory, with matching waiver entries removed. ChangesEXAONE 4.5 MTP configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 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: |
85ff940 to
6c3d236
Compare
|
Hi @BowenFu, good catch on both points. I removed the native subclass and AutoConfig re-registration entirely. With the currently pinned Transformers 5.5.4, the fallback remains the live path and keeps the normalization narrowly scoped. If native exaone4_5 support becomes available after a dependency bump, we now use HF’s config and registration unchanged, preserving the AutoModel* mappings. Also, EXAONE 4.5 33B requires approximately 66 GB of device memory, which exceeds the roughly 44 GiB available on an L40S. Skipping these QA cases on L40S is therefore intentional. |
|
/bot run --disable-fail-fast |
|
PR_Github #63726 [ run ] triggered by Bot. Commit: |
Signed-off-by: yechank <161688079+yechank-nvidia@users.noreply.github.com>
6c3d236 to
18b0abd
Compare
BowenFu
left a comment
BowenFu
left a comment
There was a problem hiding this comment.
Thanks for the update. A couple of suggestions before merge:
-
Could we remove the native
else:subclass andAutoConfig.register(..., exist_ok=True)path? The current commit still registersExaone4_5Configover HF’s nativeExaone4_5_Config, which may breakAutoModel*.from_config/from_pretrainedlookups on Transformers >=5.8. This also appears inconsistent with the latest comment saying that subclass and re-registration were removed. -
Could we scope
skip_less_device_memory(60000)to the two affected tests instead of the wholeTestExaone4_5_33Bclass? That would avoid silently skipping future tests added to this class on lower-memory GPUs.
Approving based on the author’s confirmation that these points will be addressed.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tensorrt_llm/_torch/models/modeling_exaone4_5.py (1)
41-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a parameterized type for
text_config.
text_config: dictdoes not describe the mapping. Usedict[str, object], or a project-specificTypedDictif the schema is shared. This improves static checking for the values validated below.Proposed annotation
-def _normalize_exaone4_5_mtp_layer_types(text_config: dict) -> None: +def _normalize_exaone4_5_mtp_layer_types(text_config: dict[str, object]) -> None:🤖 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` at line 41, Update the text_config parameter annotation in _normalize_exaone4_5_mtp_layer_types from an unparameterized dict to dict[str, object], or the project’s shared TypedDict if one defines this schema, while preserving the function’s existing validation behavior.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.
Inline comments:
In `@tests/integration/defs/accuracy/test_llm_api_pytorch_multimodal.py`:
- Around line 134-135: Update the test-db L0 B200 CI configuration to include
both parameterized cases of TestExaone4_5_33B.test_auto_dtype, matching the
existing entries in llm_function_core.txt; preserve the test’s current
parameterization and ensure both cases run with LLM_MODELS_ROOT configured on
GPU-enabled CI.
---
Nitpick comments:
In `@tensorrt_llm/_torch/models/modeling_exaone4_5.py`:
- Line 41: Update the text_config parameter annotation in
_normalize_exaone4_5_mtp_layer_types from an unparameterized dict to dict[str,
object], or the project’s shared TypedDict if one defines this schema, while
preserving the function’s existing validation behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d5c0d4fa-8433-4b54-baf0-8763b87a7bbd
📒 Files selected for processing (4)
tensorrt_llm/_torch/models/modeling_exaone4_5.pytests/integration/defs/accuracy/test_llm_api_pytorch_multimodal.pytests/integration/test_lists/waives.txttests/unittest/_torch/modeling/test_modeling_exaone4_5.py
💤 Files with no reviewable changes (1)
- tests/integration/test_lists/waives.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unittest/_torch/modeling/test_modeling_exaone4_5.py
|
/bot run --disable-fail-fast |
|
PR_Github #63740 [ run ] triggered by Bot. Commit: |
|
PR_Github #63726 [ run ] completed with state |
|
PR_Github #63740 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #63939 [ run ] triggered by Bot. Commit: |
|
PR_Github #63939 [ run ] completed with state |
5 participants
Dev Engineer Review
layer_typesentries.layer_typescount mismatches.AutoConfigre-registration to preserve Hugging Face mappings.TestExaone4_5_33B.forced_chunked_prefillandfull_budget.TestExaone4_5_33B.QA Engineer Review
test_exaone4_5_config_normalizes_trailing_mtp_layer_types.test_exaone4_5_config_preserves_unexpected_layer_type_mismatch.TestExaone4_5_33B.test_auto_dtypewith the device-memory skip marker.tests/integration/test_lists/waives.txt.tests/integration/test_lists/test-db/ortests/integration/test_lists/qa/.