Repository navigation
[https://nvbugs/6209806][fix] Add an idempotent _relax_llama_validate_architecture() helper that installs a pe - #14707
Closed
tensorrt-cicd wants to merge 1 commit into
Closed
tensorrt-cicd wants to merge 1 commit into
tensorrt-cicd wants to merge 1 commit into
Conversation
Contributor
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe change adds an idempotent Llama architecture validation patch and invokes it before pretrained configuration and Hugging Face tokenizer loading. ChangesLlama architecture validation
Estimated code review effort: 3 (Moderate) | ~20 minutes 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tensorrt_llm/tokenizer/tokenizer.py`:
- Around line 633-639: The compatibility patch calls
(_ensure_gpt2_bytes_to_unicode_compat and _relax_llama_validate_architecture)
are placed outside the try block in load_hf_tokenizer so import/patch failures
escape instead of being caught; move the two calls (and their import of
_relax_llama_validate_architecture from
tensorrt_llm._torch.pyexecutor.config_utils) inside the existing try: block in
load_hf_tokenizer so any ImportError or runtime error is logged and the function
returns None as intended.
🪄 Autofix (Beta)
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: 207680b0-a4cc-4c57-9764-b4cdb1c74c10
📒 Files selected for processing (3)
tensorrt_llm/_torch/pyexecutor/config_utils.pytensorrt_llm/tokenizer/tokenizer.pytests/integration/test_lists/waives.txt
💤 Files with no reviewable changes (1)
- tests/integration/test_lists/waives.txt
Comment on lines
633
to
+639
| _ensure_gpt2_bytes_to_unicode_compat() | ||
| # AutoTokenizer.from_pretrained internally calls AutoConfig.from_pretrained, | ||
| # which triggers transformers 5.x LlamaConfig.validate_architecture. | ||
| # Relax it so models with explicit `head_dim` (e.g. kanana-1.5-2.1b) load. | ||
| from tensorrt_llm._torch.pyexecutor.config_utils import \ | ||
| _relax_llama_validate_architecture | ||
| _relax_llama_validate_architecture() |
Contributor
There was a problem hiding this comment.
Move the compatibility patch inside the existing try block.
These new lines run before the try: at Line 641, so any import or patching failure now escapes instead of following load_hf_tokenizer()'s current "log and return None" behavior.
💡 Proposed fix
- _ensure_gpt2_bytes_to_unicode_compat()
- # AutoTokenizer.from_pretrained internally calls AutoConfig.from_pretrained,
- # which triggers transformers 5.x LlamaConfig.validate_architecture.
- # Relax it so models with explicit `head_dim` (e.g. kanana-1.5-2.1b) load.
- from tensorrt_llm._torch.pyexecutor.config_utils import \
- _relax_llama_validate_architecture
- _relax_llama_validate_architecture()
-
try:
+ _ensure_gpt2_bytes_to_unicode_compat()
+ # AutoTokenizer.from_pretrained internally calls AutoConfig.from_pretrained,
+ # which triggers transformers 5.x LlamaConfig.validate_architecture.
+ # Relax it so models with explicit `head_dim` (e.g. kanana-1.5-2.1b) load.
+ from tensorrt_llm._torch.pyexecutor.config_utils import \
+ _relax_llama_validate_architecture
+ _relax_llama_validate_architecture()
tokenizer = TransformersTokenizer.from_pretrained(
model_dir,
legacy=False,🤖 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/tokenizer/tokenizer.py` around lines 633 - 639, The
compatibility patch calls (_ensure_gpt2_bytes_to_unicode_compat and
_relax_llama_validate_architecture) are placed outside the try block in
load_hf_tokenizer so import/patch failures escape instead of being caught; move
the two calls (and their import of _relax_llama_validate_architecture from
tensorrt_llm._torch.pyexecutor.config_utils) inside the existing try: block in
load_hf_tokenizer so any ImportError or runtime error is logged and the function
returns None as intended.
tensorrt-cicd
force-pushed
the
repair-bot-bug6209806
branch
3 times, most recently
from
June 2, 2026 23:23
0d285b8 to
a5a48f3
Compare
dongxuy04
reviewed
Jun 3, 2026
Comment on lines
+458
to
+460
| head_dim = getattr(self, "head_dim", None) | ||
| if isinstance(head_dim, int) and head_dim > 0: | ||
| return |
Collaborator
There was a problem hiding this comment.
It seems that LlamaConfig will set head_dim if it is None here. So it might always be int and very likely larger than 0. And this seems might disable the self.hidden_size % self.num_attention_heads != 0 check for all cases.
tensorrt-cicd
force-pushed
the
repair-bot-bug6209806
branch
2 times, most recently
from
June 8, 2026 11:12
5320bf6 to
e5cc5e4
Compare
tensorrt-cicd
force-pushed
the
repair-bot-bug6209806
branch
2 times, most recently
from
June 27, 2026 04:00
6f26e1c to
2460154
Compare
trtllm-agent
force-pushed
the
repair-bot-bug6209806
branch
from
July 7, 2026 00:50
2460154 to
219063f
Compare
trtllm-agent
force-pushed
the
repair-bot-bug6209806
branch
from
July 29, 2026 07:37
219063f to
78110cc
Compare
trtllm-agent
requested review from
EmmaQiaoCh,
allisonlim-nv,
brnguyen2,
crazydemo,
lori-ren,
mlefeb01 and
schetlur-nv
July 29, 2026 07:37
Collaborator
|
/bot run --only-qa-verify test accuracy/test_llm_api_pytorch.py::TestKanana_Instruct::test_auto_dtype |
trtllm-agent
force-pushed
the
repair-bot-bug6209806
branch
from
August 3, 2026 10:39
78110cc to
7f21079
Compare
1 task done
xinhe-nv
reviewed
Aug 6, 2026
| accuracy/test_llm_api_pytorch.py::TestGPTOSS::test_w4_4gpus[v2_kv_cache-ep4-cutlass-auto] SKIP (https://nvbugs/5596343) | ||
| accuracy/test_llm_api_pytorch.py::TestGPTOSS::test_w4_4gpus[v2_kv_cache-tp4-cutlass-auto] SKIP (https://nvbugs/5596343) | ||
| accuracy/test_llm_api_pytorch.py::TestGPTOSS::test_w4_chunked_prefill[cutlass-auto] SKIP (https://nvbugs/5596343) | ||
| accuracy/test_llm_api_pytorch.py::TestKanana_Instruct::test_auto_dtype SKIP (https://nvbugs/6209806) |
Collaborator
There was a problem hiding this comment.
this model is retired, see https://docs.google.com/spreadsheets/d/17qLRr5oQESqn1rT9LrIUnpjvhH8wcC59273YEPT9k44/edit?gid=177162358#gid=177162358, should we keep it?
transformers 5.5.x LlamaConfig.validate_architecture rejects configs where hidden_size % num_attention_heads != 0, ignoring an explicit head_dim that decouples q/k/v projection size from hidden_size (e.g. kanana-1.5-2.1b: hidden_size=1792, num_attention_heads=24, head_dim=128). Replace the validator (and its captured entry in __class_validators__) with a permissive version that allows positive explicit head_dim. Apply the patch at the two TRT-LLM entry points that load HF configs and tokenizers, and remove the now-stale test waiver. Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
trtllm-agent
force-pushed
the
repair-bot-bug6209806
branch
from
August 12, 2026 18:55
7f21079 to
1f543b8
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
|
Closing this PR since model has now been retired. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Test plan
Links
Dev Engineer Review
_relax_llama_validate_architecture()adds an idempotent compatibility patch for Transformers 5.5.x.head_dimvalues whenhidden_sizeis not divisible bynum_attention_heads.load_pretrained_config()andload_hf_tokenizer()apply the patch before Transformers loads Llama configuration data.kanana-1.5-2.1b.head_dim.QA Engineer Review
No test changes.